diff mbox series

[v2,2/7] bus: Introduce firewall controller framework

Message ID 20200128153806.7780-3-benjamin.gaignard@st.com (mailing list archive)
State New, archived
Headers show
Series Introduce bus firewall controller framework | expand

Commit Message

Benjamin GAIGNARD Jan. 28, 2020, 3:38 p.m. UTC
The goal of this framework is to offer an interface for the
hardware blocks controlling bus accesses rights.

Bus firewall controllers are typically used to control if a
hardware block can perform read or write operations on bus.

Smarter firewall controllers could be able to define accesses
rights per hardware blocks to control where they can read
or write.

Firewall controller configurations are provided in device node,
parsed by the framework and send to the driver to apply them.
Each controller may need different number and type of inputs
to configure the firewall so device-tree properties size have to
be define by using "#firewall-cells".
Firewall configurations properties have to be named "firewall-X"
on device node.
"firewall-names" keyword can also be used to give a name to
a specific configuration.

Example of device-tree:
ctrl0: firewall@0 {
	#firewall-cells = <2>;
      };

foo: foo@0 {
	firewall-names = "default", "setting1";
	firewall-0 = <&ctrl0 1 2>;
	firewall-1 = <&ctrl0 3 4>;
};

Configurations could be applied with functions like
firewall_set_config_by_index() or firewall_set_config_by_name().

firewall_set_default_config() function will apply the
configuration named "default" (if existing) or the configuration
with index 0 (i.e. firewall-0).

Drivers could register/unregister themselves be calling
firewall_register/firewall_unregister functions.

When a configuration has to be applied the driver callback,
provided in the ops at registration time, set_config is called
by the framework.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
version 2:
- rename the framework "firewall"
- rebased on top of v5.5

 drivers/bus/Kconfig             |   2 +
 drivers/bus/Makefile            |   2 +
 drivers/bus/firewall/Kconfig    |   7 ++
 drivers/bus/firewall/Makefile   |   1 +
 drivers/bus/firewall/firewall.c | 264 ++++++++++++++++++++++++++++++++++++++++
 include/linux/firewall.h        |  70 +++++++++++
 6 files changed, 346 insertions(+)
 create mode 100644 drivers/bus/firewall/Kconfig
 create mode 100644 drivers/bus/firewall/Makefile
 create mode 100644 drivers/bus/firewall/firewall.c
 create mode 100644 include/linux/firewall.h

Comments

Greg KH Jan. 28, 2020, 3:52 p.m. UTC | #1
On Tue, Jan 28, 2020 at 04:38:01PM +0100, Benjamin Gaignard wrote:
> The goal of this framework is to offer an interface for the
> hardware blocks controlling bus accesses rights.
> 
> Bus firewall controllers are typically used to control if a
> hardware block can perform read or write operations on bus.

So put this in the bus-specific code that controls the bus that these
devices live on.  Why put it in the driver core when this is only on one
"bus" (i.e. the catch-all-and-a-bag-of-chips platform bus)?

And really, this should just be a totally new bus type, right?  And any
devices on this bus should be changed to be on this new bus, and the
drivers changed to support them, instead of trying to overload the
platform bus with more stuff.

thanks,

greg k-h
Benjamin GAIGNARD Jan. 28, 2020, 4:41 p.m. UTC | #2
On 1/28/20 4:52 PM, Greg KH wrote:
> On Tue, Jan 28, 2020 at 04:38:01PM +0100, Benjamin Gaignard wrote:
>> The goal of this framework is to offer an interface for the
>> hardware blocks controlling bus accesses rights.
>>
>> Bus firewall controllers are typically used to control if a
>> hardware block can perform read or write operations on bus.
> So put this in the bus-specific code that controls the bus that these
> devices live on.  Why put it in the driver core when this is only on one
> "bus" (i.e. the catch-all-and-a-bag-of-chips platform bus)?

It is really similar to what pin controller does, configuring an 
hardware block given DT information.

I could argue that firewalls are not bus themselves they only interact 
with it.

Bus firewalls exist on other SoC, I hope some others could be added in 
this framework. ETZPC is only the first.

>
> And really, this should just be a totally new bus type, right?  And any
> devices on this bus should be changed to be on this new bus, and the
> drivers changed to support them, instead of trying to overload the
> platform bus with more stuff.

I have tried to use the bus notifier to avoid to add this code at probe 
time but without success:

https://lkml.org/lkml/2018/2/27/300

I have also tried to disable the nodes at runtime and Mark Rutland 
explain me why it was wrong.

Benjamin

>
> thanks,
>
> greg k-h
Greg KH Jan. 28, 2020, 4:57 p.m. UTC | #3
On Tue, Jan 28, 2020 at 04:41:29PM +0000, Benjamin GAIGNARD wrote:
> 
> On 1/28/20 4:52 PM, Greg KH wrote:
> > On Tue, Jan 28, 2020 at 04:38:01PM +0100, Benjamin Gaignard wrote:
> >> The goal of this framework is to offer an interface for the
> >> hardware blocks controlling bus accesses rights.
> >>
> >> Bus firewall controllers are typically used to control if a
> >> hardware block can perform read or write operations on bus.
> > So put this in the bus-specific code that controls the bus that these
> > devices live on.  Why put it in the driver core when this is only on one
> > "bus" (i.e. the catch-all-and-a-bag-of-chips platform bus)?
> 
> It is really similar to what pin controller does, configuring an 
> hardware block given DT information.

Great, then use that instead :)

> I could argue that firewalls are not bus themselves they only interact 
> with it.

They live on a bus, and do so in bus-specific ways, right?

> Bus firewalls exist on other SoC, I hope some others could be added in 
> this framework. ETZPC is only the first.

Then put it on the bus it lives on, and the bus that the drivers for
that device are being controlled with.  That sounds like the sane place
to do so, right?

> > And really, this should just be a totally new bus type, right?  And any
> > devices on this bus should be changed to be on this new bus, and the
> > drivers changed to support them, instead of trying to overload the
> > platform bus with more stuff.
> 
> I have tried to use the bus notifier to avoid to add this code at probe 
> time but without success:
> 
> https://lkml.org/lkml/2018/2/27/300

Almost 2 years ago?  I can't remember something written 1 week ago...

Yes, don't abuse the notifier chain.  I hate that thing as it is.

> I have also tried to disable the nodes at runtime and Mark Rutland 
> explain me why it was wrong.

The bus controller should do this, right?  Why not just do it there?

thanks,

greg k-h
Benjamin GAIGNARD Jan. 28, 2020, 8:29 p.m. UTC | #4
On 1/28/20 5:57 PM, Greg KH wrote:
> On Tue, Jan 28, 2020 at 04:41:29PM +0000, Benjamin GAIGNARD wrote:
>> On 1/28/20 4:52 PM, Greg KH wrote:
>>> On Tue, Jan 28, 2020 at 04:38:01PM +0100, Benjamin Gaignard wrote:
>>>> The goal of this framework is to offer an interface for the
>>>> hardware blocks controlling bus accesses rights.
>>>>
>>>> Bus firewall controllers are typically used to control if a
>>>> hardware block can perform read or write operations on bus.
>>> So put this in the bus-specific code that controls the bus that these
>>> devices live on.  Why put it in the driver core when this is only on one
>>> "bus" (i.e. the catch-all-and-a-bag-of-chips platform bus)?
>> It is really similar to what pin controller does, configuring an
>> hardware block given DT information.
> Great, then use that instead :)
I think that Linus W. will complain if I do that :)
>
>> I could argue that firewalls are not bus themselves they only interact
>> with it.
> They live on a bus, and do so in bus-specific ways, right?
>
>> Bus firewalls exist on other SoC, I hope some others could be added in
>> this framework. ETZPC is only the first.
> Then put it on the bus it lives on, and the bus that the drivers for
> that device are being controlled with.  That sounds like the sane place
> to do so, right?

If that means that all drivers have to be modified it will be 
problematic because not all

are specifics to the SoC.

>
>>> And really, this should just be a totally new bus type, right?  And any
>>> devices on this bus should be changed to be on this new bus, and the
>>> drivers changed to support them, instead of trying to overload the
>>> platform bus with more stuff.
>> I have tried to use the bus notifier to avoid to add this code at probe
>> time but without success:
>>
>> https://lkml.org/lkml/2018/2/27/300
> Almost 2 years ago?  I can't remember something written 1 week ago...
>
> Yes, don't abuse the notifier chain.  I hate that thing as it is.
>
>> I have also tried to disable the nodes at runtime and Mark Rutland
>> explain me why it was wrong.
> The bus controller should do this, right?  Why not just do it there?

The bus controller is a different hardware block.


>
> thanks,
>
> greg k-h
Greg KH Jan. 29, 2020, 5:49 a.m. UTC | #5
On Tue, Jan 28, 2020 at 08:29:45PM +0000, Benjamin GAIGNARD wrote:
> 
> On 1/28/20 5:57 PM, Greg KH wrote:
> > On Tue, Jan 28, 2020 at 04:41:29PM +0000, Benjamin GAIGNARD wrote:
> >> On 1/28/20 4:52 PM, Greg KH wrote:
> >>> On Tue, Jan 28, 2020 at 04:38:01PM +0100, Benjamin Gaignard wrote:
> >>>> The goal of this framework is to offer an interface for the
> >>>> hardware blocks controlling bus accesses rights.
> >>>>
> >>>> Bus firewall controllers are typically used to control if a
> >>>> hardware block can perform read or write operations on bus.
> >>> So put this in the bus-specific code that controls the bus that these
> >>> devices live on.  Why put it in the driver core when this is only on one
> >>> "bus" (i.e. the catch-all-and-a-bag-of-chips platform bus)?
> >> It is really similar to what pin controller does, configuring an
> >> hardware block given DT information.
> > Great, then use that instead :)
> I think that Linus W. will complain if I do that :)
> >
> >> I could argue that firewalls are not bus themselves they only interact
> >> with it.
> > They live on a bus, and do so in bus-specific ways, right?
> >
> >> Bus firewalls exist on other SoC, I hope some others could be added in
> >> this framework. ETZPC is only the first.
> > Then put it on the bus it lives on, and the bus that the drivers for
> > that device are being controlled with.  That sounds like the sane place
> > to do so, right?
> 
> If that means that all drivers have to be modified it will be 
> problematic because not all
> 
> are specifics to the SoC.

That's fine, we have loads of drivers that work on different types of
busses.

Or, if this really is the "platform bus" then use that.  (which is what
I was hinting at all along but no one seems to realize that, should have
been more obvious...)

> >>> And really, this should just be a totally new bus type, right?  And any
> >>> devices on this bus should be changed to be on this new bus, and the
> >>> drivers changed to support them, instead of trying to overload the
> >>> platform bus with more stuff.
> >> I have tried to use the bus notifier to avoid to add this code at probe
> >> time but without success:
> >>
> >> https://lkml.org/lkml/2018/2/27/300
> > Almost 2 years ago?  I can't remember something written 1 week ago...
> >
> > Yes, don't abuse the notifier chain.  I hate that thing as it is.
> >
> >> I have also tried to disable the nodes at runtime and Mark Rutland
> >> explain me why it was wrong.
> > The bus controller should do this, right?  Why not just do it there?
> 
> The bus controller is a different hardware block.

Of course it is, but it controls a bus, and there are devices on that
bus, right?  Don't circumvent things please.

greg k-h
Linus Walleij Jan. 29, 2020, 9:42 a.m. UTC | #6
On Tue, Jan 28, 2020 at 9:30 PM Benjamin GAIGNARD
<benjamin.gaignard@st.com> wrote:
> On 1/28/20 5:57 PM, Greg KH wrote:
> > On Tue, Jan 28, 2020 at 04:41:29PM +0000, Benjamin GAIGNARD wrote:
> >> On 1/28/20 4:52 PM, Greg KH wrote:

> >>> So put this in the bus-specific code that controls the bus that these
> >>> devices live on.  Why put it in the driver core when this is only on one
> >>> "bus" (i.e. the catch-all-and-a-bag-of-chips platform bus)?

> >> It is really similar to what pin controller does, configuring an
> >> hardware block given DT information.

> > Great, then use that instead :)

> I think that Linus W. will complain if I do that :)

So the similarity would be something like the way that pin control
states are configured in the device tree and the pin control
handles are taken before probe in drivers/base/pinctrl.c embedding
a hook into dd.c.

Not that it in any way controls any hardware even remotely
similar to pin control. Pin control is an electronic thing,
this firewalling is about bus access.

IIUC this framework wants to discover at kernel boot time
whether certain devices are accessible to it or not by inspecting
the state of the firewalling hardware and then avoid probing
those that are inaccessible.

It needs the same deep hooks into dd.c to achieve this
I believe.

Yours,
Linus Walleij
Greg KH Jan. 29, 2020, 9:52 a.m. UTC | #7
On Wed, Jan 29, 2020 at 10:42:39AM +0100, Linus Walleij wrote:
> On Tue, Jan 28, 2020 at 9:30 PM Benjamin GAIGNARD
> <benjamin.gaignard@st.com> wrote:
> > On 1/28/20 5:57 PM, Greg KH wrote:
> > > On Tue, Jan 28, 2020 at 04:41:29PM +0000, Benjamin GAIGNARD wrote:
> > >> On 1/28/20 4:52 PM, Greg KH wrote:
> 
> > >>> So put this in the bus-specific code that controls the bus that these
> > >>> devices live on.  Why put it in the driver core when this is only on one
> > >>> "bus" (i.e. the catch-all-and-a-bag-of-chips platform bus)?
> 
> > >> It is really similar to what pin controller does, configuring an
> > >> hardware block given DT information.
> 
> > > Great, then use that instead :)
> 
> > I think that Linus W. will complain if I do that :)
> 
> So the similarity would be something like the way that pin control
> states are configured in the device tree and the pin control
> handles are taken before probe in drivers/base/pinctrl.c embedding
> a hook into dd.c.
> 
> Not that it in any way controls any hardware even remotely
> similar to pin control. Pin control is an electronic thing,
> this firewalling is about bus access.
> 
> IIUC this framework wants to discover at kernel boot time
> whether certain devices are accessible to it or not by inspecting
> the state of the firewalling hardware and then avoid probing
> those that are inaccessible.
> 
> It needs the same deep hooks into dd.c to achieve this
> I believe.

It just needs to be part of the bus logic for the specific bus that this
"firewall" is on.  Just like we do the same thing for USB or thunderbolt
devices.  Put this in the bus-specific code please.

thanks,

greg k-h
Mark Brown Jan. 29, 2020, 11:17 a.m. UTC | #8
On Wed, Jan 29, 2020 at 10:52:40AM +0100, Greg KH wrote:

> It just needs to be part of the bus logic for the specific bus that this
> "firewall" is on.  Just like we do the same thing for USB or thunderbolt
> devices.  Put this in the bus-specific code please.

I'd expect that this is going to affect at least platform and AMBA
buses.
Benjamin GAIGNARD Jan. 31, 2020, 8:37 a.m. UTC | #9
On 1/29/20 12:17 PM, Mark Brown wrote:
> On Wed, Jan 29, 2020 at 10:52:40AM +0100, Greg KH wrote:
>
>> It just needs to be part of the bus logic for the specific bus that this
>> "firewall" is on.  Just like we do the same thing for USB or thunderbolt
>> devices.  Put this in the bus-specific code please.
> I'd expect that this is going to affect at least platform and AMBA
> buses.

Correct me if I'm wrong but creating a new type of bus would mean

that all the drivers living on this bus must be changed to register 
themselves on this bus ?

Or does a solution exist to let them live on the platform bus and call 
firewalled bus before been probed ?

All the impacted drivers could work on the existing bus with or without 
the firewall so I don't want to break

that.

Benjamin
Greg KH Jan. 31, 2020, 9:06 a.m. UTC | #10
On Fri, Jan 31, 2020 at 08:37:27AM +0000, Benjamin GAIGNARD wrote:
> 
> On 1/29/20 12:17 PM, Mark Brown wrote:
> > On Wed, Jan 29, 2020 at 10:52:40AM +0100, Greg KH wrote:
> >
> >> It just needs to be part of the bus logic for the specific bus that this
> >> "firewall" is on.  Just like we do the same thing for USB or thunderbolt
> >> devices.  Put this in the bus-specific code please.
> > I'd expect that this is going to affect at least platform and AMBA
> > buses.
> 
> Correct me if I'm wrong but creating a new type of bus would mean
> that all the drivers living on this bus must be changed to register 
> themselves on this bus ?

Yes.

> Or does a solution exist to let them live on the platform bus and call 
> firewalled bus before been probed ?

Why do people want to abuse the platform bus so much?  If a device is on
a bus that can have such a controller, then it is on a real bus, use it!

> All the impacted drivers could work on the existing bus with or without 
> the firewall so I don't want to break

break what?

> 
> that.

Odd line-break :)

Just register the driver on both busses, no big deal.

Stop abusing the platform bus code for things that it is not for.

thanks,

greg k-h
Linus Walleij Feb. 14, 2020, 4:05 p.m. UTC | #11
On Fri, Jan 31, 2020 at 10:06 AM Greg KH <gregkh@linuxfoundation.org> wrote:

> Why do people want to abuse the platform bus so much?  If a device is on
> a bus that can have such a controller, then it is on a real bus, use it!

I'm not saying it is a good thing, but the reason why it is (ab)used so
much can be found in:
drivers/of/platform.c

TL;DR: struct platform_device is the Device McDeviceFace and
platform bus the Bus McBusFace used by the device tree parser since
it is slightly to completely unaware of what devices it is actually
spawning.

And everything and its dog is using device tree in the embedded
world. (A quick glance in drivers/acpi gives me the impression
that ACPI is doing the very same thing but I am not a domain expert
there so I am not really sure.)

Whenever a device is created from a device tree it gets spawned
on either the platform bus or the amba bus. In 99 cases out of
100 it is going to be a platform_device.

In most device trees all devices ultimately spawn from the device
tree and the root of absolutely everything including irq chips on
the SoC, timers, PCI hosts and USB root hubs and whatnot is a
platform device, because that is how the core device tree parser has
chosen to spawn off devices.

This generic code goes back to
commit eca3930163ba8884060ce9d9ff5ef0d9b7c7b00f
"of: Merge of_platform_bus_type with platform_bus_type"
where the device tree-specific bus was replaced by the
platform bus. This code was then moved down to drivers/of
and used in multiple architectures. Grant's patch makes perfect
sense because at the time some devices were created using board
files (thus platform_device) and others using device tree and having
two different probe paths and driver files for this reason alone
was not reasonable. The same reasoning will apply to ACPI
vs device tree drivers.

What we  *could* have done was to handle special devices
special, like happened for AMBA PrimeCells. Mea Culpa, I suppose
I am one of the guilty.

Supporting new bus types for root devices in systems described
in device tree would requiring patching drivers/of/platform.c
and people are afraid of that because the code there is pretty
complex.

Instead platform_device is (ab)used to carry stuff over from the
device tree to respective subsystem.

In some cases the struct platform_device from device tree is
discarded after use, it is just left dangling in memory with no other
purpose than to serve as .parent for whatever device on whatever
bus we were really creating.

For some devices such as root irq_chips they serve no purpose
whatsoever, they are just created and sitting around never
to be probed, because the code instantiating them parse the
device tree directly.

For the devices that actually probe to drive a piece of silicon,
arguably a different type of device on a different bus should be
created, such as (I am making this up) struct soc_device
on soc_bus. (Incidentally soc_bus exists, but its current use case
is not for this.)

I don't really see any better option for Benjamin or anyone else
though?

The reason why it is used so much should at least be clarified
now I think.

Yours,
Linus Walleij
Greg KH Feb. 14, 2020, 9:40 p.m. UTC | #12
On Fri, Feb 14, 2020 at 05:05:07PM +0100, Linus Walleij wrote:
> On Fri, Jan 31, 2020 at 10:06 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> 
> > Why do people want to abuse the platform bus so much?  If a device is on
> > a bus that can have such a controller, then it is on a real bus, use it!
> 
> I'm not saying it is a good thing, but the reason why it is (ab)used so
> much can be found in:
> drivers/of/platform.c
> 
> TL;DR: struct platform_device is the Device McDeviceFace and
> platform bus the Bus McBusFace used by the device tree parser since
> it is slightly to completely unaware of what devices it is actually
> spawning.

<snip>

Yeah, great explaination, and I understand.  DT stuff really is ok to be
on a platform bus, as that's what almost all of them are.

But, when you try to start messing around with things like this
"firewall" says it is doing, it's then obvious that this really isn't a
DT like thing, but rather you do have a bus involved with a controller
so that should be used instead.

Or just filter away the DT stuff so that the kernel never even sees
those devices, which might just be simplest :)

thanks,

greg k-h
Benjamin GAIGNARD Feb. 15, 2020, 12:41 p.m. UTC | #13
On 2/14/20 10:40 PM, Greg KH wrote:
> On Fri, Feb 14, 2020 at 05:05:07PM +0100, Linus Walleij wrote:
>> On Fri, Jan 31, 2020 at 10:06 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>>
>>> Why do people want to abuse the platform bus so much?  If a device is on
>>> a bus that can have such a controller, then it is on a real bus, use it!
>> I'm not saying it is a good thing, but the reason why it is (ab)used so
>> much can be found in:
>> drivers/of/platform.c
>>
>> TL;DR: struct platform_device is the Device McDeviceFace and
>> platform bus the Bus McBusFace used by the device tree parser since
>> it is slightly to completely unaware of what devices it is actually
>> spawning.
> <snip>
>
> Yeah, great explaination, and I understand.  DT stuff really is ok to be
> on a platform bus, as that's what almost all of them are.
>
> But, when you try to start messing around with things like this
> "firewall" says it is doing, it's then obvious that this really isn't a
> DT like thing, but rather you do have a bus involved with a controller
> so that should be used instead.

Ok but how put in place a new bus while keeping the devices on platform
bus to avoid changing all the drivers ?

>
> Or just filter away the DT stuff so that the kernel never even sees
> those devices, which might just be simplest :)

yes but we lost the possibility to change the firewall configuration at
run time. I do expect to be able to describe in the DT firewall configuration
and to use them at run time. That could allow, for example, to handover
a HW block to the coprocessor when the main core is going to be suspended
to save power.

Benjamin

>
> thanks,
>
> greg k-h
Greg KH Feb. 16, 2020, 7:21 a.m. UTC | #14
On Sat, Feb 15, 2020 at 12:41:07PM +0000, Benjamin GAIGNARD wrote:
> 
> On 2/14/20 10:40 PM, Greg KH wrote:
> > On Fri, Feb 14, 2020 at 05:05:07PM +0100, Linus Walleij wrote:
> >> On Fri, Jan 31, 2020 at 10:06 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >>
> >>> Why do people want to abuse the platform bus so much?  If a device is on
> >>> a bus that can have such a controller, then it is on a real bus, use it!
> >> I'm not saying it is a good thing, but the reason why it is (ab)used so
> >> much can be found in:
> >> drivers/of/platform.c
> >>
> >> TL;DR: struct platform_device is the Device McDeviceFace and
> >> platform bus the Bus McBusFace used by the device tree parser since
> >> it is slightly to completely unaware of what devices it is actually
> >> spawning.
> > <snip>
> >
> > Yeah, great explaination, and I understand.  DT stuff really is ok to be
> > on a platform bus, as that's what almost all of them are.
> >
> > But, when you try to start messing around with things like this
> > "firewall" says it is doing, it's then obvious that this really isn't a
> > DT like thing, but rather you do have a bus involved with a controller
> > so that should be used instead.
> 
> Ok but how put in place a new bus while keeping the devices on platform
> bus to avoid changing all the drivers ?

You don't, you put them all on your real bus, as that is what you now
have.

> > Or just filter away the DT stuff so that the kernel never even sees
> > those devices, which might just be simplest :)
> 
> yes but we lost the possibility to change the firewall configuration at
> run time. I do expect to be able to describe in the DT firewall configuration
> and to use them at run time. That could allow, for example, to handover
> a HW block to the coprocessor when the main core is going to be suspended
> to save power.

Then use a real bus :)

thanks,

greg k-h
diff mbox series

Patch

diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index 50200d1c06ea..d3f636c64e1c 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -203,4 +203,6 @@  config DA8XX_MSTPRI
 
 source "drivers/bus/fsl-mc/Kconfig"
 
+source "drivers/bus/firewall/Kconfig"
+
 endmenu
diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
index 1320bcf9fa9d..278c27fd57cd 100644
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -34,3 +34,5 @@  obj-$(CONFIG_UNIPHIER_SYSTEM_BUS)	+= uniphier-system-bus.o
 obj-$(CONFIG_VEXPRESS_CONFIG)	+= vexpress-config.o
 
 obj-$(CONFIG_DA8XX_MSTPRI)	+= da8xx-mstpri.o
+
+obj-$(CONFIG_FIREWALL_CONTROLLERS) 	+= firewall/
diff --git a/drivers/bus/firewall/Kconfig b/drivers/bus/firewall/Kconfig
new file mode 100644
index 000000000000..893bacb955f5
--- /dev/null
+++ b/drivers/bus/firewall/Kconfig
@@ -0,0 +1,7 @@ 
+menu "Bus Firewall Controllers"
+
+config FIREWALL_CONTROLLERS
+	bool "Support of bus firewall controllers"
+	depends on OF
+
+endmenu
diff --git a/drivers/bus/firewall/Makefile b/drivers/bus/firewall/Makefile
new file mode 100644
index 000000000000..eb6b978d6450
--- /dev/null
+++ b/drivers/bus/firewall/Makefile
@@ -0,0 +1 @@ 
+obj-$(CONFIG_FIREWALL_CONTROLLERS) += firewall.o
diff --git a/drivers/bus/firewall/firewall.c b/drivers/bus/firewall/firewall.c
new file mode 100644
index 000000000000..765105d29075
--- /dev/null
+++ b/drivers/bus/firewall/firewall.c
@@ -0,0 +1,264 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) STMicroelectronics 2020 - All Rights Reserved
+ * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
+ */
+
+#include <linux/device.h>
+#include <linux/firewall.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+
+/* Mutex taken to protect firewall_list */
+static DEFINE_MUTEX(firewall_list_mutex);
+
+/* Global list of firewall control devices */
+static LIST_HEAD(firewall_list);
+
+struct firewall_ctrl {
+	struct list_head node;
+	struct device *dev;
+	struct firewall_ops *ops;
+};
+
+static struct firewall_ctrl *get_firewallctrl_from_node(struct device_node *np)
+{
+	struct firewall_ctrl *ctrl;
+
+	mutex_lock(&firewall_list_mutex);
+
+	list_for_each_entry(ctrl, &firewall_list, node) {
+		if (ctrl->dev->of_node == np) {
+			mutex_unlock(&firewall_list_mutex);
+			return ctrl;
+		}
+	}
+
+	mutex_unlock(&firewall_list_mutex);
+
+	return NULL;
+}
+
+/**
+ * firewall_dt_has_default
+ *
+ * Check if the device node provide firewall configuration
+ *
+ * @dev: device with possible firewall configuration
+ *
+ * Return: true is firewall-0 property exist in the device node
+ */
+static bool firewall_dt_has_default(struct device *dev)
+{
+	struct device_node *np;
+	struct property *prop;
+	int size;
+
+	np = dev->of_node;
+	if (!np)
+		return false;
+
+	prop = of_find_property(np, "firewall-0", &size);
+
+	return prop ? true : false;
+}
+
+/**
+ * firewall_set_config_by_index
+ *
+ * Set a firewall controller configuration based on given index.
+ *
+ * @dev: device with firewall configuration to apply.
+ * @index: the index of the configuration in device node.
+ *
+ * Return: 0 if OK, -EPROBE_DEFER if waiting for firewall controller to be
+ * registered or negative value on other errors.
+ */
+int firewall_set_config_by_index(struct device *dev, int index)
+{
+	struct device_node *np = dev->of_node;
+	char *propname;
+	int configs, i, err = 0;
+
+	if (!np)
+		return 0;
+
+	propname = kasprintf(GFP_KERNEL, "firewall-%d", index);
+	configs = of_count_phandle_with_args(np, propname, "#firewall-cells");
+	if (configs < 0) {
+		err = -EINVAL;
+		goto error;
+	}
+
+	for (i = 0; i < configs; i++) {
+		struct firewall_ctrl *ctrl;
+		struct of_phandle_args args;
+
+		err = of_parse_phandle_with_args(np, propname,
+						 "#firewall-cells",
+						 i, &args);
+		if (err)
+			goto error;
+
+		/* Test if the controller is (or will be) available */
+		if (!of_device_is_available(args.np)) {
+			of_node_put(args.np);
+			continue;
+		}
+
+		ctrl = get_firewallctrl_from_node(args.np);
+		of_node_put(args.np);
+
+		/* Controller is not yet registered */
+		if (!ctrl) {
+			err = -EPROBE_DEFER;
+			goto error;
+		}
+
+		err = ctrl->ops->set_config(ctrl->dev, &args);
+		if (err)
+			goto error;
+	}
+
+error:
+	kfree(propname);
+	return err;
+}
+EXPORT_SYMBOL_GPL(firewall_set_config_by_index);
+
+/**
+ * firewall_set_config_by_name
+ *
+ * Set a firwall controller configuration based on given name.
+ *
+ * @dev: device with firewall configuration to apply.
+ * @name: the name of the configuration in device node.
+ *
+ * Return: 0 if OK, -EPROBE_DEFER if waiting for firewall controller to be
+ * registered or negative value on other errors.
+ */
+int firewall_set_config_by_name(struct device *dev, char *name)
+{
+	const char *configname;
+	int count, i;
+
+	count = of_property_count_strings(dev->of_node, "firewall-names");
+	for (i = 0; i < count; i++) {
+		int err;
+
+		err = of_property_read_string_index(dev->of_node,
+						    "firewall-names",
+						    i, &configname);
+		if (err)
+			return err;
+
+		if (strcmp(name, configname))
+			continue;
+
+		return firewall_set_config_by_index(dev, i);
+	}
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(firewall_set_config_by_name);
+
+/**
+ * firewall_set_default_config
+ *
+ * Set the default configuration for device.
+ * First try to apply configuration named "default", if it fails
+ * or doesn't exist, try to apply firewall-0 configuration.
+ *
+ * @dev: device with firewall configuration to apply.
+ *
+ * Return: 0 if OK, -EPROBE_DEFER if waiting for firewall controller to be
+ * registered or negative value on other errors.
+ */
+int firewall_set_default_config(struct device *dev)
+{
+	int ret;
+
+	/* Nothing to do if device node doesn't contain at least
+	 * one configuration
+	 */
+	if (!firewall_dt_has_default(dev))
+		return 0;
+
+	ret = firewall_set_config_by_name(dev, "default");
+	if (!ret || (ret == -EPROBE_DEFER))
+		return ret;
+
+	return firewall_set_config_by_index(dev, 0);
+}
+EXPORT_SYMBOL_GPL(firewall_set_default_config);
+
+/**
+ * firewall_register
+ *
+ * Register a firewall controller device.
+ *
+ * @dev: device implementing firewall controller.
+ * @ops: firewall controller operations.
+ *
+ * Return: 0 if OK or negative value on error.
+ */
+int firewall_register(struct device *dev,
+		      struct firewall_ops *ops)
+{
+	struct firewall_ctrl *ctrl;
+
+	if (!dev || !ops || !ops->set_config)
+		return -EINVAL;
+
+	ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL);
+	if (!ctrl)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&ctrl->node);
+
+	ctrl->dev = dev;
+	ctrl->ops = ops;
+
+	mutex_lock(&firewall_list_mutex);
+	list_add_tail(&ctrl->node, &firewall_list);
+	mutex_unlock(&firewall_list_mutex);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(firewall_register);
+
+/**
+ * firewall_unregister
+ *
+ * Unregister a firewall controller device.
+ *
+ * @dev: device implementing firewall controller.
+ */
+void firewall_unregister(struct device *dev)
+{
+	struct firewall_ctrl *ctrl;
+
+	ctrl = get_firewallctrl_from_node(dev->of_node);
+	if (!ctrl)
+		return;
+
+	mutex_lock(&firewall_list_mutex);
+	list_del(&ctrl->node);
+	mutex_unlock(&firewall_list_mutex);
+
+	kfree(ctrl);
+}
+EXPORT_SYMBOL_GPL(firewall_unregister);
+
+static int __init firewall_init(void)
+{
+	pr_info("initialized bus firewall controller subsystem\n");
+	return 0;
+}
+
+/* Init early since drivers really need to configure firewall early */
+core_initcall(firewall_init);
diff --git a/include/linux/firewall.h b/include/linux/firewall.h
new file mode 100644
index 000000000000..67eb9985821c
--- /dev/null
+++ b/include/linux/firewall.h
@@ -0,0 +1,70 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) STMicroelectronics 2020 - All Rights Reserved
+ * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
+ */
+
+#ifndef _FIREWALL_H_
+#define _FIREWALL_H_
+
+#include <linux/device.h>
+#include <linux/of.h>
+
+/**
+ * struct firewall_ops
+ *
+ * Firewall controller operations structure to be filled by drivers.
+ */
+struct firewall_ops {
+	/**
+	 * @set_config:
+	 *
+	 * Driver callback to set a firewall configuration on a controller.
+	 * Configuration arguments are provided in out_args parameter.
+	 *
+	 * Return: 0 on success, a negative error code on failure.
+	 */
+	int (*set_config)(struct device *dev, struct of_phandle_args *out_args);
+};
+
+#ifdef CONFIG_FIREWALL_CONTROLLERS
+
+int firewall_set_config_by_index(struct device *dev, int index);
+int firewall_set_config_by_name(struct device *dev, char *name);
+int firewall_set_default_config(struct device *dev);
+
+int firewall_register(struct device *dev, struct firewall_ops *ops);
+
+void firewall_unregister(struct device *dev);
+
+#else
+
+static inline int firewall_set_config_by_index(struct device *dev, int index)
+{
+	return 0;
+}
+
+static inline int firewall_set_config_by_name(struct device *dev, char *name)
+{
+	return 0;
+}
+
+static inline int firewall_set_default_config(struct device *dev)
+{
+	return 0;
+}
+
+static inline int firewall_register(struct device *dev,
+				    struct firewall_ops *ops)
+{
+	return 0;
+}
+
+static inline void firewall_unregister(struct device *dev)
+{
+	/* Empty */
+}
+
+#endif
+
+#endif /* _FIREWALL_H_ */