Messages in this thread | | | Date | Fri, 7 Jul 2023 16:00:23 +0200 | Subject | Re: [PATCH 05/10] firewall: introduce stm32_firewall framework | From | Gatien CHEVALLIER <> |
| |
Hello Greg,
On 7/7/23 12:37, Greg KH wrote: > On Wed, Jul 05, 2023 at 07:27:54PM +0200, Gatien Chevallier wrote: >> 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
>> >> 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.
>> >> 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? > >> + 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.
>> + >> 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
| |