Re: [PATCH 05/10] firewall: introduce stm32_firewall framework

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


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,


> Best regards,
> Gatien


[Index of Archives]     [Linux Memonry Technology]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux