Hello Greg,

On 7/7/23 12:37, Greg KH wrote:

On Wed, Jul 05, 2023 at 07:27:54PM +0200, Gatien Chevallier wrote:

quoted

Introduce a 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 firewall framework offers a generic API that is defined in firewall
controllers drivers to best fit the specificity of each firewall.

But you aren't defining a "generic" api here, you are defining a
specific one for your specific hardware.

Or am I missing something?

This framework is indeed ST-oriented, I should have specified :
"...offers a generic API for STM32 firewall controllers that is defined
in their drivers...". Will change in V2

quoted

There are various types of firewalls:
-Peripheral firewalls that filter accesses to peripherals
-Memory firewalls that filter accesses to memories or memory regions
-Resource firewalls that filter accesses to internal resources such as
reset and clock controllers

A firewall controller must be probed at arch_initcall level and register
to the framework so that consumers can use their services.

Why must it happen at arch_initcall?  So it can never be a module?  That
feels wrong.

Rob asked the same question.

I responded under his comment, I'm not quite sure how to handle it with
fw_devlink for dependencies with drivers that are probed at early init
levels.

quoted

Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
---
  MAINTAINERS                               |   5 +
  arch/arm64/Kconfig.platforms              |   1 +
  drivers/bus/Kconfig                       |  10 +
  drivers/bus/Makefile                      |   1 +
  drivers/bus/stm32_firewall.c              | 252 ++++++++++++++++++++++
  drivers/bus/stm32_firewall.h              |  83 +++++++
  include/linux/bus/stm32_firewall_device.h | 134 ++++++++++++
  7 files changed, 486 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 41385f01fa98..fabf95ba9b86 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS

@@ -20123,6 +20123,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 <gatien.chevallier@foss.st.com>
+S:	Maintained
+F:	drivers/bus/stm32_firewall.c
+
  ST STM32 I2C/SMBUS DRIVER
  M:	Pierre-Yves MORDRET <pierre-yves.mordret@foss.st.com>
  M:	Alain Volmat <alain.volmat@foss.st.com>

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 fcfa280df98a..4d54a7ea52b2 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig

@@ -163,6 +163,16 @@ 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

Why this dependency?

quoted

+	default MACH_STM32MP157 || MACH_STM32MP13 || MACH_STM32MP25
+	help
+	  Say y to enable firewall framework and its services. Firewall
+	  controllers will be able to register to the framework. Firewall
+	  controllers must be initialized and register to the firewall framework
+	  at arch_initcall level.

This needs better wording saying it is only for stm32 devices.

Ack, will change in V2.

quoted

+
  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..510db5bc6eaf
--- /dev/null
+++ b/drivers/bus/stm32_firewall.c

@@ -0,0 +1,252 @@
+// SPDX-License-Identifier: GPL-2.0-or-later

Are you _SURE_ this needs to be "or later"?  Sorry, I have to ask.

thanks,

greg k-h

I'll change to (GPL-2.0-only OR BSD-3-Clause) :)

Best regards,
Gatien