Hello Gatien, Gatien CHEVALLIER <gatien.chevallier@xxxxxxxxxxx> writes: > Hello Rob, > > On 7/7/23 17:07, Rob Herring wrote: >> On Fri, Jul 07, 2023 at 03:43:15PM +0200, Gatien CHEVALLIER wrote: >>> >>> >>> On 7/6/23 17:09, Rob Herring 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. >>>>> >>>>> 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 >>>> >>>> How do resource firewalls work? Access to registers for some clocks in a >>>> clock controller are disabled? Or something gates off clocks/resets to >>>> a block? >>> >>> To take a practical example: >>> >>> A clock controller can be firewall-aware and have its own firewall registers >>> to configure. To access a clock/reset that is handled this way, a device >>> would need to check this "resource firewall". I thought that for these kinds >>> of hardware blocks, having a common API would help. >> We already have the concept of 'protected clocks' which are ones >> controlled by secure mode which limits what Linux can do with them. I >> think you should extend this mechanism if needed and use the existing >> clock/reset APIs for managing resources. >> > > Ok, thank you for the input. I'll remove this type of firewall for V2 as > I no longer have a use case. > >>>> >>>> It might make more sense for "resource" accesses to be managed within >>>> those resource APIs (i.e. the clock and reset frameworks) and leave this >>>> framework to bus accesses. >>>> >>> >>> Okay, I'll drop this for V2 if you find that the above explaination do not >>> justify this. >>> >>>>> A firewall controller must be probed at arch_initcall level and register >>>>> to the framework so that consumers can use their services. >>>> >>>> initcall ordering hacks should not be needed. We have both deferred >>>> probe and fw_devlinks to avoid that problem. >>>> >>> >>> Greg also doubts this. >>> >>> Drivers like reset/clock controllers drivers (core_initcall level) will have >>> a dependency on the firewall controllers in order to initialize their >>> resources. I was not sure how to manage these dependencies. >>> >>> Now, looking at init/main.c, I've realized that core_initcall() level comes >>> before arch_initcall() level... >>> >>> If managed by fw_devlink, the feature-domains property should be supported >>> as well I suppose? I'm not sure how to handle this properly. I'd welcome >>> your suggestion. >> DT parent/child child dependencies are already handled which might >> be >> enough for you. Otherwise, adding a new provider/consumer binding is a >> couple of lines to add the property names. See drivers/of/property.c. >> > > Ok, I'll try with a modification of drivers/of/property.c as the > parent/child dependency won't be enough. Thanks for pointing this out. > >>>>> Signed-off-by: Gatien Chevallier <gatien.chevallier@xxxxxxxxxxx> >>>>> --- >>>>> 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 +++++++ >>>> >>>> Why something stm32 specific? We know there are multiple platforms >>>> wanting something in this area. Wasn't the last attempt common? >>>> >>>> For a common binding, I'm not eager to accept anything new with only 1 >>>> user. >>>> >>> >>> Last attempt was common for the feature-domain bindings. The system-bus >>> driver was ST-specific. I don't know if other platforms needs this kind >>> of framework. Are you suggesting that this framework should be generic? Or >>> that this framework should have a st-specific property? >> Ah right, the posting for SCMI device permissions was the binding >> only. >> The binding should be generic and support more than 1 user. That somewhat >> implies a generic framework, but not necessarily. >> >>> I've oriented this firewall framework to serve ST purpose. There may be a >>> need for other platforms but I'm not sure that this framework serves them >>> well. One can argue that it is quite minimalist and covers basic purposes of >>> a hardware firewall but I would need more feedback from other vendors to >>> submit it as a generic one. >> We already know there are at least 2 users. Why would we make the >> 2nd >> user refactor your driver into a common framework? >> [...] >> > > If one thinks this framework is generic enough so it can be of use for > them, so yes, I can submit it as a common framework. I'm not that sure > Oleksii finds a use case with it. He seemed interested by the bindings. > Maybe I'm wrong Oleksii? > Correct. I'm interested only in bindings which should be processed by the hypervisor and removed from the OS DT the Kernel running in VM wouldn't know it exists. > For V2, I'd rather submit it again as an ST-specific framework again to > address the generic comments. This way, other people have time to > manifest themselves. > >>>>> +int stm32_firewall_get_firewall(struct device_node *np, [snip] > > Best regards, > Gatien -- Thanks, Oleksii
