mbox series

[v2,00/11] Introduce STM32 Firewall framework

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

Message

Gatien CHEVALLIER July 25, 2023, 4:40 p.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 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: bus: document RIFSC
  dt-bindings: bus: document ETZPC
  dt-bindings: treewide: add feature-domains description
  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                  |  288 ++
 drivers/bus/stm32_firewall.h                  |   83 +
 drivers/bus/stm32_rifsc.c                     |  252 ++
 drivers/of/property.c                         |    2 +
 include/linux/bus/stm32_firewall_device.h     |  140 +
 47 files changed, 3346 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

Rob Herring July 25, 2023, 5:49 p.m. UTC | #1
On Tue, 25 Jul 2023 18:40:56 +0200, Gatien Chevallier wrote:
> Document ETZPC (Extended TrustZone protection controller). ETZPC is a
> firewall controller.
> 
> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
> ---
> 
> Changes in V2:
> 	- Corrected errors highlighted by Rob's robot
> 	- No longer define the maxItems for the "feature-domains"
> 	  property
> 	- Fix example (node name, status)
> 	- Declare "feature-domain-names" as an optional
> 	  property for child nodes
> 	- Fix description of "feature-domains" property
> 	- Reorder the properties so it matches RIFSC
> 	- Add missing "feature-domain-controller" property
> 
>  .../bindings/bus/st,stm32-etzpc.yaml          | 96 +++++++++++++++++++
>  1 file changed, 96 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/bus/st,stm32-etzpc.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/bus/st,stm32-etzpc.example.dtb: serial@4c001000: Unevaluated properties are not allowed ('feature-domains' was unexpected)
	from schema $id: http://devicetree.org/schemas/serial/st,stm32-uart.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230725164104.273965-4-gatien.chevallier@foss.st.com

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.
Simon Horman July 26, 2023, 10:22 a.m. UTC | #2
On Tue, Jul 25, 2023 at 06:40:58PM +0200, Gatien Chevallier wrote:

...

> diff --git a/drivers/bus/stm32_firewall.c b/drivers/bus/stm32_firewall.c

...

> +int stm32_firewall_controller_register(struct stm32_firewall_controller *firewall_controller)
> +{
> +	struct stm32_firewall_controller *ctrl;
> +
> +	pr_info("Registering %s firewall controller\n", firewall_controller->name);
> +
> +	if (!firewall_controller)
> +		return -ENODEV;

HI Gatien,

Sorry, one more on this patch, that I missed before sending my previous
email.

firewall_controller is checked for NULL here.
But it is already dereferenced on the line above the check.

Flagged by Smatch.

...
Gatien CHEVALLIER July 26, 2023, 10:38 a.m. UTC | #3
On 7/26/23 12:19, Simon Horman wrote:
> On Tue, Jul 25, 2023 at 06:40:58PM +0200, Gatien Chevallier wrote:
>> 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 <gatien.chevallier@foss.st.com>
> 
> ...
> 
>> diff --git a/drivers/bus/stm32_firewall.c b/drivers/bus/stm32_firewall.c
> 
> ...
> 
>> +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 feature-domains property is mandatory for firewall bus devices */
>> +		len = of_count_phandle_with_args(child, "feature-domains", "#feature-domain-cells");
>> +		if (len <= 0)
> 
> Coccinelle says that, due to breaking out of a
> for_each_available_child_of_node() loop, a call to of_node_put()
> is needed here
> 

Hi Simon,

Thank you, I already sent a V3 correcting the patch ordering issue. I
will implement this for V4.

>> +			return -EINVAL;
>> +
>> +		firewalls = kcalloc(len, sizeof(*firewalls), GFP_KERNEL);
>> +		if (!firewalls)
> 
> And here.
> 

ditto

>> +			return -ENOMEM;
>> +
>> +		err = stm32_firewall_get_firewall(child, firewalls, (unsigned int)len);
>> +		if (err) {
>> +			kfree(firewalls);
> 
> And here.
> 

ditto

>> +			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
> 
> ...
> 
>> +/**
>> + * struct stm32_firewall_controller - Information on firewall controller supplying services
>> + *
>> + * @name			Name of the firewall controller
> 
> kernel-doc complains that name and the other fields of
> struct stm32_firewall_controller are not documented.
> I believe this is because a ':' is needed after the name of
> the parameter (in this case 'name').
> 
>   * @name:			Name of the firewall controller
> 
> Likewise, elsewhere.
> 

I will implement it in V4, thank you.

>> + * @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);
>> +};
>> +
>> +/**
>> + * int stm32_firewall_controller_register - Register a firewall controller to the STM32 firewall
> 
> kernel-doc seems unhappy about the presence of 'int' on this line.
> 
>   * stm32_firewall_controller_register - Register a firewall controller to the STM32 firewall
> 
> Likewise, elsewhere.
> 

Yes, I will remove the type in V4.

>> + *					    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);
> 
> ...
> 
>> diff --git a/include/linux/bus/stm32_firewall_device.h b/include/linux/bus/stm32_firewall_device.h
>> new file mode 100644
>> index 000000000000..9bdc4060154c
>> --- /dev/null
>> +++ b/include/linux/bus/stm32_firewall_device.h
>> @@ -0,0 +1,140 @@
>> +/* 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;
>> +
>> +/**
>> + * stm32_firewall - Information on a device's firewall. Each device can have more than one firewall.
> 
> kernel-doc seems unhappy about the absence of struct on this line.
> 
>   * struct stm32_firewall - Information on a device's firewall. Each device can have more than one firewall.
> 

Yes, I will add the struct in V4.

>> + *
>> + * @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;
>> +};
> 
> ...
> 
Best regards,
Gatien
Gatien CHEVALLIER July 26, 2023, 10:39 a.m. UTC | #4
On 7/26/23 12:22, Simon Horman wrote:
> On Tue, Jul 25, 2023 at 06:40:58PM +0200, Gatien Chevallier wrote:
> 
> ...
> 
>> diff --git a/drivers/bus/stm32_firewall.c b/drivers/bus/stm32_firewall.c
> 
> ...
> 
>> +int stm32_firewall_controller_register(struct stm32_firewall_controller *firewall_controller)
>> +{
>> +	struct stm32_firewall_controller *ctrl;
>> +
>> +	pr_info("Registering %s firewall controller\n", firewall_controller->name);
>> +
>> +	if (!firewall_controller)
>> +		return -ENODEV;
> 
> HI Gatien,
> 
> Sorry, one more on this patch, that I missed before sending my previous
> email.
> 
> firewall_controller is checked for NULL here.
> But it is already dereferenced on the line above the check.
> 
> Flagged by Smatch.
> 
> ...

Indeed, thank you. I will change this for V4

Best regards,
Gatien
Simon Horman July 26, 2023, 11:39 a.m. UTC | #5
On Wed, Jul 26, 2023 at 12:38:00PM +0200, Gatien CHEVALLIER wrote:
> 
> 
> On 7/26/23 12:19, Simon Horman wrote:
> > On Tue, Jul 25, 2023 at 06:40:58PM +0200, Gatien Chevallier wrote:
> > > 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 <gatien.chevallier@foss.st.com>
> > 
> > ...
> > 
> > > diff --git a/drivers/bus/stm32_firewall.c b/drivers/bus/stm32_firewall.c
> > 
> > ...
> > 
> > > +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 feature-domains property is mandatory for firewall bus devices */
> > > +		len = of_count_phandle_with_args(child, "feature-domains", "#feature-domain-cells");
> > > +		if (len <= 0)
> > 
> > Coccinelle says that, due to breaking out of a
> > for_each_available_child_of_node() loop, a call to of_node_put()
> > is needed here
> > 
> 
> Hi Simon,
> 
> Thank you, I already sent a V3 correcting the patch ordering issue. I
> will implement this for V4.

Hi Gatien,

Thanks and sorry for not noticing v3 - I think it arrived while
I was in the middle of reviewing v2.