From: Benjamin GAIGNARD <benjamin.gaignard@st.com> To: Florian Fainelli <f.fainelli@gmail.com>, Robin Murphy <robin.murphy@arm.com>, Sudeep Holla <sudeep.holla@arm.com> Cc: Mark Rutland <mark.rutland@arm.com>, "robh@kernel.org" <robh@kernel.org>, Loic PALLARDY <loic.pallardy@st.com>, "arnd@arndb.de" <arnd@arndb.de>, "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>, "s.hauer@pengutronix.de" <s.hauer@pengutronix.de>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "system-dt@lists.openampproject.org" <system-dt@lists.openampproject.org>, "broonie@kernel.org" <broonie@kernel.org>, "lkml@metux.net" <lkml@metux.net>, "linux-imx@nxp.com" <linux-imx@nxp.com>, "kernel@pengutronix.de" <kernel@pengutronix.de>, "fabio.estevam@nxp.com" <fabio.estevam@nxp.com>, "stefano.stabellini@xilinx.com" <stefano.stabellini@xilinx.com>, "shawnguo@kernel.org" <shawnguo@kernel.org>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH v2 0/7] Introduce bus firewall controller framework Date: Mon, 3 Feb 2020 13:41:55 +0000 [thread overview] Message-ID: <d8b41083-9dfa-5fb8-ecd7-d12151a29aad@st.com> (raw) In-Reply-To: <e370fb7a-02a6-f5f3-c87d-cd09a80d69ec@gmail.com> On 1/31/20 9:51 PM, Florian Fainelli wrote: > On 1/29/20 5:40 AM, Benjamin GAIGNARD wrote: >> On 1/28/20 11:06 PM, Robin Murphy wrote: >>> On 2020-01-28 8:06 pm, Benjamin GAIGNARD wrote: >>>> On 1/28/20 6:17 PM, Sudeep Holla wrote: >>>>> On Tue, Jan 28, 2020 at 04:46:41PM +0000, Benjamin GAIGNARD wrote: >>>>>> On 1/28/20 5:36 PM, Sudeep Holla wrote: >>>>>>> On Tue, Jan 28, 2020 at 04:37:59PM +0100, Benjamin Gaignard wrote: >>>>>>>> Bus firewall framework aims to provide a kernel API to set the >>>>>>>> configuration >>>>>>>> of the harware blocks in charge of busses access control. >>>>>>>> >>>>>>>> Framework architecture is inspirated by pinctrl framework: >>>>>>>> - a default configuration could be applied before bind the driver. >>>>>>>> If a configuration could not be applied the driver is not bind >>>>>>>> to avoid doing accesses on prohibited regions. >>>>>>>> - configurations could be apllied dynamically by drivers. >>>>>>>> - device node provides the bus firewall configurations. >>>>>>>> >>>>>>>> An example of bus firewall controller is STM32 ETZPC hardware block >>>>>>>> which got 3 possible configurations: >>>>>>>> - trust: hardware blocks are only accessible by software running >>>>>>>> on trust >>>>>>>> zone (i.e op-tee firmware). >>>>>>>> - non-secure: hardware blocks are accessible by non-secure >>>>>>>> software (i.e. >>>>>>>> linux kernel). >>>>>>>> - coprocessor: hardware blocks are only accessible by the >>>>>>>> coprocessor. >>>>>>>> Up to 94 hardware blocks of the soc could be managed by ETZPC. >>>>>>>> >>>>>>> /me confused. Is ETZPC accessible from the non-secure kernel space to >>>>>>> begin with ? If so, is it allowed to configure hardware blocks as >>>>>>> secure >>>>>>> or trusted ? I am failing to understand the overall design of a >>>>>>> system >>>>>>> with ETZPC controller. >>>>>> Non-secure kernel could read the values set in ETZPC, if it doesn't >>>>>> match >>>>>> with what is required by the device node the driver won't be probed. >>>>>> >>>>> OK, but I was under the impression that it was made clear that Linux is >>>>> not firmware validation suite. The firmware need to ensure all the >>>>> devices >>>>> that are not accessible in the Linux kernel are marked as disabled and >>>>> this needs to happen before entering the kernel. So if this is what >>>>> this >>>>> patch series achieves, then there is no need for it. Please stop >>>>> pursuing >>>>> this any further or provide any other reasons(if any) to have it. Until >>>>> you have other reasons, NACK for this series. >>>> No it doesn't disable the nodes. >>>> >>>> When the firmware disable a node before the kernel that means it change >>>> >>>> the DTB and that is a problem when you want to sign it. With my proposal >>>> >>>> the DTB remains the same. >>> ??? >>> >>> :/ >>> >>> The DTB is used to pass the kernel command line, memory reservations, >>> random seeds, and all manner of other things dynamically generated by >>> firmware at boot-time. Apologies for being blunt but if "changing the >>> DTB" is considered a problem then I can't help but think you're doing >>> it wrong. >> Yes but I would like to limit the number of cases where a firmware has >> to change the DTB. >> >> With this proposal nodes remain the same and embedded the firewall >> configuration(s). >> >> Until now firewall configuration is "static", the firmware disable (or >> remove) the nodes not accessible from Linux. >> >> If Linux can rely on node's firewall information it could allow switch >> dynamically an hardware block from Linux to a coprocessor. >> >> For example Linux could manage the display pipe configuration and when >> going to suspend handover the display hardware block to a coprocessor in >> charge a refreshing only some pixels. > OK, let's continue that example, would not it make sense then to just > steal the peripheral away from Linux by ensuring that Linux is no longer > running and the only thing that you need to make sure of is that either > you restore the HW in the exact same that you stole it from, or that > Linux is capable of refreshing its state against what the HW state was > left in? > > If you have a set of display pipeline drivers, on your way to suspend, > you can define a protocol with the co-processor so as to signal an > ownership change, and the co-processor can take control from there. To handover a hardware block to the co-processor we need to inform it and change the firewall configuration. My proposal only aim to cover this last point by describing in the device tree the possible configuration. The example I had mind is how the pinctrl framework is working with it states so doing something like changing firewall configuration and then inform the co-processor in suspend function. > > In your example, it sounds like the firewall could be meant to detect > uncoordinated concurrent accesses to the same HW block between different > SW/FW entities. If that is the case, this is most likely a bug and you > can probably just get away with doing reporting instead of an entirely > new subsystem? Prohibited accesses, most of the time, generate an abort on the bus so your platform just crash and yes it is a bug. This new subsystem won't change that, it only allow to describe and dynamically set a configuration for DT information rather doing that for type of firewall. Benjamin
WARNING: multiple messages have this Message-ID (diff)
From: Benjamin GAIGNARD <benjamin.gaignard@st.com> To: Florian Fainelli <f.fainelli@gmail.com>, Robin Murphy <robin.murphy@arm.com>, Sudeep Holla <sudeep.holla@arm.com> Cc: Mark Rutland <mark.rutland@arm.com>, "robh@kernel.org" <robh@kernel.org>, Loic PALLARDY <loic.pallardy@st.com>, "arnd@arndb.de" <arnd@arndb.de>, "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>, "s.hauer@pengutronix.de" <s.hauer@pengutronix.de>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "broonie@kernel.org" <broonie@kernel.org>, "lkml@metux.net" <lkml@metux.net>, "linux-imx@nxp.com" <linux-imx@nxp.com>, "kernel@pengutronix.de" <kernel@pengutronix.de>, "fabio.estevam@nxp.com" <fabio.estevam@nxp.com>, "shawnguo@kernel.org" <shawnguo@kernel.org>, "stefano.stabellini@xilinx.com" <stefano.stabellini@xilinx.com>, "system-dt@lists.openampproject.org" <system-dt@lists.openampproject.org>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH v2 0/7] Introduce bus firewall controller framework Date: Mon, 3 Feb 2020 13:41:55 +0000 [thread overview] Message-ID: <d8b41083-9dfa-5fb8-ecd7-d12151a29aad@st.com> (raw) In-Reply-To: <e370fb7a-02a6-f5f3-c87d-cd09a80d69ec@gmail.com> On 1/31/20 9:51 PM, Florian Fainelli wrote: > On 1/29/20 5:40 AM, Benjamin GAIGNARD wrote: >> On 1/28/20 11:06 PM, Robin Murphy wrote: >>> On 2020-01-28 8:06 pm, Benjamin GAIGNARD wrote: >>>> On 1/28/20 6:17 PM, Sudeep Holla wrote: >>>>> On Tue, Jan 28, 2020 at 04:46:41PM +0000, Benjamin GAIGNARD wrote: >>>>>> On 1/28/20 5:36 PM, Sudeep Holla wrote: >>>>>>> On Tue, Jan 28, 2020 at 04:37:59PM +0100, Benjamin Gaignard wrote: >>>>>>>> Bus firewall framework aims to provide a kernel API to set the >>>>>>>> configuration >>>>>>>> of the harware blocks in charge of busses access control. >>>>>>>> >>>>>>>> Framework architecture is inspirated by pinctrl framework: >>>>>>>> - a default configuration could be applied before bind the driver. >>>>>>>> If a configuration could not be applied the driver is not bind >>>>>>>> to avoid doing accesses on prohibited regions. >>>>>>>> - configurations could be apllied dynamically by drivers. >>>>>>>> - device node provides the bus firewall configurations. >>>>>>>> >>>>>>>> An example of bus firewall controller is STM32 ETZPC hardware block >>>>>>>> which got 3 possible configurations: >>>>>>>> - trust: hardware blocks are only accessible by software running >>>>>>>> on trust >>>>>>>> zone (i.e op-tee firmware). >>>>>>>> - non-secure: hardware blocks are accessible by non-secure >>>>>>>> software (i.e. >>>>>>>> linux kernel). >>>>>>>> - coprocessor: hardware blocks are only accessible by the >>>>>>>> coprocessor. >>>>>>>> Up to 94 hardware blocks of the soc could be managed by ETZPC. >>>>>>>> >>>>>>> /me confused. Is ETZPC accessible from the non-secure kernel space to >>>>>>> begin with ? If so, is it allowed to configure hardware blocks as >>>>>>> secure >>>>>>> or trusted ? I am failing to understand the overall design of a >>>>>>> system >>>>>>> with ETZPC controller. >>>>>> Non-secure kernel could read the values set in ETZPC, if it doesn't >>>>>> match >>>>>> with what is required by the device node the driver won't be probed. >>>>>> >>>>> OK, but I was under the impression that it was made clear that Linux is >>>>> not firmware validation suite. The firmware need to ensure all the >>>>> devices >>>>> that are not accessible in the Linux kernel are marked as disabled and >>>>> this needs to happen before entering the kernel. So if this is what >>>>> this >>>>> patch series achieves, then there is no need for it. Please stop >>>>> pursuing >>>>> this any further or provide any other reasons(if any) to have it. Until >>>>> you have other reasons, NACK for this series. >>>> No it doesn't disable the nodes. >>>> >>>> When the firmware disable a node before the kernel that means it change >>>> >>>> the DTB and that is a problem when you want to sign it. With my proposal >>>> >>>> the DTB remains the same. >>> ??? >>> >>> :/ >>> >>> The DTB is used to pass the kernel command line, memory reservations, >>> random seeds, and all manner of other things dynamically generated by >>> firmware at boot-time. Apologies for being blunt but if "changing the >>> DTB" is considered a problem then I can't help but think you're doing >>> it wrong. >> Yes but I would like to limit the number of cases where a firmware has >> to change the DTB. >> >> With this proposal nodes remain the same and embedded the firewall >> configuration(s). >> >> Until now firewall configuration is "static", the firmware disable (or >> remove) the nodes not accessible from Linux. >> >> If Linux can rely on node's firewall information it could allow switch >> dynamically an hardware block from Linux to a coprocessor. >> >> For example Linux could manage the display pipe configuration and when >> going to suspend handover the display hardware block to a coprocessor in >> charge a refreshing only some pixels. > OK, let's continue that example, would not it make sense then to just > steal the peripheral away from Linux by ensuring that Linux is no longer > running and the only thing that you need to make sure of is that either > you restore the HW in the exact same that you stole it from, or that > Linux is capable of refreshing its state against what the HW state was > left in? > > If you have a set of display pipeline drivers, on your way to suspend, > you can define a protocol with the co-processor so as to signal an > ownership change, and the co-processor can take control from there. To handover a hardware block to the co-processor we need to inform it and change the firewall configuration. My proposal only aim to cover this last point by describing in the device tree the possible configuration. The example I had mind is how the pinctrl framework is working with it states so doing something like changing firewall configuration and then inform the co-processor in suspend function. > > In your example, it sounds like the firewall could be meant to detect > uncoordinated concurrent accesses to the same HW block between different > SW/FW entities. If that is the case, this is most likely a bug and you > can probably just get away with doing reporting instead of an entirely > new subsystem? Prohibited accesses, most of the time, generate an abort on the bus so your platform just crash and yes it is a bug. This new subsystem won't change that, it only allow to describe and dynamically set a configuration for DT information rather doing that for type of firewall. Benjamin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-02-03 13:42 UTC|newest] Thread overview: 68+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-01-28 15:37 [PATCH v2 0/7] Introduce bus firewall controller framework Benjamin Gaignard 2020-01-28 15:37 ` Benjamin Gaignard 2020-01-28 15:38 ` [PATCH v2 1/7] dt-bindings: bus: Add firewall bindings Benjamin Gaignard 2020-01-28 15:38 ` Benjamin Gaignard 2020-01-28 15:38 ` [PATCH v2 2/7] bus: Introduce firewall controller framework Benjamin Gaignard 2020-01-28 15:38 ` Benjamin Gaignard 2020-01-28 15:52 ` Greg KH 2020-01-28 15:52 ` Greg KH 2020-01-28 16:41 ` Benjamin GAIGNARD 2020-01-28 16:41 ` Benjamin GAIGNARD 2020-01-28 16:57 ` Greg KH 2020-01-28 16:57 ` Greg KH 2020-01-28 20:29 ` Benjamin GAIGNARD 2020-01-28 20:29 ` Benjamin GAIGNARD 2020-01-29 5:49 ` Greg KH 2020-01-29 5:49 ` Greg KH 2020-01-29 9:42 ` Linus Walleij 2020-01-29 9:42 ` Linus Walleij 2020-01-29 9:52 ` Greg KH 2020-01-29 9:52 ` Greg KH 2020-01-29 11:17 ` Mark Brown 2020-01-29 11:17 ` Mark Brown 2020-01-31 8:37 ` Benjamin GAIGNARD 2020-01-31 8:37 ` Benjamin GAIGNARD 2020-01-31 9:06 ` Greg KH 2020-01-31 9:06 ` Greg KH 2020-02-14 16:05 ` Linus Walleij 2020-02-14 16:05 ` Linus Walleij 2020-02-14 21:40 ` Greg KH 2020-02-14 21:40 ` Greg KH 2020-02-15 12:41 ` Benjamin GAIGNARD 2020-02-15 12:41 ` Benjamin GAIGNARD 2020-02-16 7:21 ` Greg KH 2020-02-16 7:21 ` Greg KH 2020-01-28 15:38 ` [PATCH v2 3/7] base: Add calls to firewall controller Benjamin Gaignard 2020-01-28 15:38 ` Benjamin Gaignard 2020-01-28 15:38 ` [PATCH v2 4/7] dt-bindings: bus: Add STM32 ETZPC " Benjamin Gaignard 2020-01-28 15:38 ` Benjamin Gaignard 2020-01-28 15:38 ` [PATCH v2 5/7] bus: firewall: Add driver for STM32 ETZPC controller Benjamin Gaignard 2020-01-28 15:38 ` Benjamin Gaignard 2020-01-28 15:38 ` [PATCH v2 6/7] ARM: dts: stm32: Add firewall node for stm32mp157 SoC Benjamin Gaignard 2020-01-28 15:38 ` Benjamin Gaignard 2020-01-28 15:38 ` [PATCH v2 7/7] ARM: dts: stm32: enable firewall controller node on stm32mp157c-ed1 Benjamin Gaignard 2020-01-28 15:38 ` Benjamin Gaignard 2020-01-28 16:36 ` [PATCH v2 0/7] Introduce bus firewall controller framework Sudeep Holla 2020-01-28 16:36 ` Sudeep Holla 2020-01-28 16:46 ` Benjamin GAIGNARD 2020-01-28 16:46 ` Benjamin GAIGNARD 2020-01-28 17:17 ` Sudeep Holla 2020-01-28 17:17 ` Sudeep Holla 2020-01-28 20:06 ` Benjamin GAIGNARD 2020-01-28 20:06 ` Benjamin GAIGNARD 2020-01-28 22:06 ` Robin Murphy 2020-01-28 22:06 ` Robin Murphy 2020-01-29 13:40 ` Benjamin GAIGNARD 2020-01-29 13:40 ` Benjamin GAIGNARD 2020-01-31 18:25 ` Robin Murphy 2020-01-31 18:25 ` Robin Murphy 2020-02-03 13:16 ` Benjamin GAIGNARD 2020-02-03 13:16 ` Benjamin GAIGNARD 2020-01-31 20:51 ` Florian Fainelli 2020-01-31 20:51 ` Florian Fainelli 2020-02-03 13:41 ` Benjamin GAIGNARD [this message] 2020-02-03 13:41 ` Benjamin GAIGNARD 2020-01-31 20:48 ` Florian Fainelli 2020-01-31 20:48 ` Florian Fainelli 2020-02-02 12:23 ` Mark Brown 2020-02-02 12:23 ` Mark Brown
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=d8b41083-9dfa-5fb8-ecd7-d12151a29aad@st.com \ --to=benjamin.gaignard@st.com \ --cc=arnd@arndb.de \ --cc=broonie@kernel.org \ --cc=devicetree@vger.kernel.org \ --cc=f.fainelli@gmail.com \ --cc=fabio.estevam@nxp.com \ --cc=gregkh@linuxfoundation.org \ --cc=kernel@pengutronix.de \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-imx@nxp.com \ --cc=linux-kernel@vger.kernel.org \ --cc=lkml@metux.net \ --cc=loic.pallardy@st.com \ --cc=mark.rutland@arm.com \ --cc=robh@kernel.org \ --cc=robin.murphy@arm.com \ --cc=s.hauer@pengutronix.de \ --cc=shawnguo@kernel.org \ --cc=stefano.stabellini@xilinx.com \ --cc=sudeep.holla@arm.com \ --cc=system-dt@lists.openampproject.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.