| Message ID | 20230705172759.1610753-1-gatien.chevallier@foss.st.com |
|---|---|
| Headers | show |
| Series | Introduce STM32 Firewall framework | expand |
On Wed, 05 Jul 2023 19:27:51 +0200, Gatien Chevallier wrote: > Document RIFSC (RIF security controller). RIFSC is a firewall controller > composed of different kinds of hardware resources. > > Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com> > --- > .../bindings/bus/st,stm32-rifsc.yaml | 101 ++++++++++++++++++ > 1 file changed, 101 insertions(+) > create mode 100644 Documentation/devicetree/bindings/bus/st,stm32-rifsc.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/bus/st,stm32-rifsc.yaml: title: 'STM32 Resource isolation framework security controller bindings' should not be valid under {'pattern': '([Bb]inding| [Ss]chema)'} hint: Everything is a binding/schema, no need to say it. Describe what hardware the binding is for. from schema $id: http://devicetree.org/meta-schemas/core.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230705172759.1610753-3-gatien.chevallier@foss.st.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Hello Krzysztof, Firstly, I will correct the bindings error pointed by Rob's robot. Obviously, I did not pass the bindings check the proper way or maybe I'm running an old version. On 7/6/23 08:28, Krzysztof Kozlowski wrote: > On 05/07/2023 19:27, Gatien Chevallier wrote: >> Document RIFSC (RIF security controller). RIFSC is a firewall controller >> composed of different kinds of hardware resources. >> >> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com> > > A nit, subject: drop second/last, redundant "device tree bindings for". > The "dt-bindings" prefix is already stating that these are bindings. 4 > words of your 6 word subject is meaningless... Ack, I will rephrase, it is indeed redundant > >> --- >> .../bindings/bus/st,stm32-rifsc.yaml | 101 ++++++++++++++++++ >> 1 file changed, 101 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/bus/st,stm32-rifsc.yaml >> >> diff --git a/Documentation/devicetree/bindings/bus/st,stm32-rifsc.yaml b/Documentation/devicetree/bindings/bus/st,stm32-rifsc.yaml >> new file mode 100644 >> index 000000000000..68d585ed369c >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/bus/st,stm32-rifsc.yaml > > Filename like compatible, unless you know list of compatibles will > grow... but then add them. > >> @@ -0,0 +1,101 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/bus/st,stm32-rifsc.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: STM32 Resource isolation framework security controller bindings > > Drop bindings Ack > >> + >> +maintainers: >> + - Gatien Chevallier <gatien.chevallier@foss.st.com> >> + >> +description: | >> + Resource isolation framework (RIF) is a comprehensive set of hardware blocks >> + designed to enforce and manage isolation of STM32 hardware resources like >> + memory and peripherals. >> + >> + The RIFSC (RIF security controller) is composed of three sets of registers, >> + each managing a specific set of hardware resources: >> + - RISC registers associated with RISUP logic (resource isolation device unit >> + for peripherals), assign all non-RIF aware peripherals to zero, one or >> + any security domains (secure, privilege, compartment). >> + - RIMC registers: associated with RIMU logic (resource isolation master >> + unit), assign all non RIF-aware bus master to one security domain by >> + setting secure, privileged and compartment information on the system bus. >> + Alternatively, the RISUP logic controlling the device port access to a >> + peripheral can assign target bus attributes to this peripheral master port >> + (supported attribute: CID). >> + - RISC registers associated with RISAL logic (resource isolation device unit >> + for address space - Lite version), assign address space subregions to one >> + security domains (secure, privilege, compartment). >> + >> +properties: >> + compatible: >> + const: st,stm32mp25-rifsc >> + >> + reg: >> + maxItems: 1 >> + >> + "#address-cells": >> + const: 1 >> + >> + "#size-cells": >> + const: 1 >> + >> + "#feature-domain-cells": >> + const: 1 >> + >> + ranges: true >> + >> + feature-domain-controller: true >> + >> +patternProperties: >> + "^.*@[0-9a-f]+$": >> + description: Peripherals >> + type: object >> + properties: >> + feature-domains: >> + minItems: 1 >> + maxItems: 2 >> + description: >> + The first argument must always be a phandle that references to the >> + firewall controller of the peripheral. The second can contain the >> + platform specific firewall ID of the peripheral. > > It does not make much sense to me to have hierarchy parent-child and via > phandle at the same time. You express the similar relationship twice Thank you for pointing this out. About the parent-child relation: The bus-like device tree architecture allows a bus-probe mechanism with which we want to check accesses of peripherals before probing their driver. This has several advantages: -This bus architecture provides a clearer view of the hardware. -No peripheral driver modifications as it is fully handled by the firewall drivers. -Drivers for devices that aren't accessible will not even be probed => no probe fail. It would be possible to manage this mechanism another way by handling probe deferrals in drivers. But it would mean modifying every driver with a check on ST firewall that we probe and some of them aren't from STMicroelectronics. About the phandle relation: I agree on the fact that this double expression of the relationship is redundant. I've done it this way because there will be other nodes outside the RIFSC node that will need to reference it as their feature-domain controller. I kept the same information in the property to be coherent between all. For nodes under the RIFSC, the phandle is indeed useless and could be removed, just to leave the firewall ID. And I'm inclined to do so. I just have one worry on the YAML binding files where I will have a pattern property in the RIFSC that will state something and maybe another description in the peripheral YAML files. What is your take on that? > >> + >> +required: >> + - compatible >> + - reg >> + - "#address-cells" >> + - "#size-cells" >> + - feature-domain-controller >> + - "#feature-domain-cells" >> + - ranges >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + // In this example, the usart2 device refers to rifsc as its domain >> + // controller. >> + // Access rights are verified before creating devices. >> + >> + #include <dt-bindings/interrupt-controller/arm-gic.h> >> + >> + rifsc: rifsc-bus@42080000 { >> + compatible = "st,stm32mp25-rifsc"; >> + reg = <0x42080000 0x1000>; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + ranges; >> + feature-domain-controller; >> + #feature-domain-cells = <1>; >> + >> + usart2: serial@400e0000 { >> + compatible = "st,stm32h7-uart"; >> + reg = <0x400e0000 0x400>; >> + interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>; >> + clocks = <&ck_flexgen_08>; >> + feature-domains = <&rifsc 32>; >> + status = "disabled"; > > No status in the examples. > >> + }; >> + }; > > Best regards, > Krzysztof > Best regards, Gatien
On Wed, Jul 05, 2023 at 07:27:53PM +0200, Gatien Chevallier wrote: > feature-domains is an optional property that allows a peripheral to > refer to one or more feature domain controller(s). > > Description of this property is added to all peripheral binding files of > the peripheral under the STM32 firewall controllers. It allows an accurate > representation of the hardware, where various peripherals are connected > to this firewall bus. The firewall can then check the peripheral accesses > before allowing it to probe. > > Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com> > --- > > Disclaimer: Some error with dtbs_check will be observed as I've > considered the property to be generic, as Rob asked > > Documentation/devicetree/bindings/crypto/st,stm32-hash.yaml | 4 ++++ > Documentation/devicetree/bindings/dma/st,stm32-dma.yaml | 4 ++++ > Documentation/devicetree/bindings/dma/st,stm32-dmamux.yaml | 4 ++++ > Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml | 4 ++++ > Documentation/devicetree/bindings/iio/adc/st,stm32-adc.yaml | 4 ++++ > .../devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml | 4 ++++ > Documentation/devicetree/bindings/iio/dac/st,stm32-dac.yaml | 4 ++++ > .../devicetree/bindings/media/cec/st,stm32-cec.yaml | 4 ++++ > Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml | 4 ++++ > .../bindings/memory-controllers/st,stm32-fmc2-ebi.yaml | 4 ++++ > Documentation/devicetree/bindings/mfd/st,stm32-lptimer.yaml | 4 ++++ > Documentation/devicetree/bindings/mfd/st,stm32-timers.yaml | 5 +++++ > Documentation/devicetree/bindings/mmc/arm,pl18x.yaml | 4 ++++ > Documentation/devicetree/bindings/net/stm32-dwmac.yaml | 4 ++++ > Documentation/devicetree/bindings/phy/phy-stm32-usbphyc.yaml | 4 ++++ > .../devicetree/bindings/regulator/st,stm32-vrefbuf.yaml | 4 ++++ > Documentation/devicetree/bindings/rng/st,stm32-rng.yaml | 4 ++++ > Documentation/devicetree/bindings/serial/st,stm32-uart.yaml | 4 ++++ > Documentation/devicetree/bindings/sound/st,stm32-i2s.yaml | 4 ++++ > Documentation/devicetree/bindings/sound/st,stm32-sai.yaml | 4 ++++ > .../devicetree/bindings/sound/st,stm32-spdifrx.yaml | 4 ++++ > Documentation/devicetree/bindings/spi/st,stm32-qspi.yaml | 4 ++++ > Documentation/devicetree/bindings/spi/st,stm32-spi.yaml | 4 ++++ > Documentation/devicetree/bindings/usb/dwc2.yaml | 4 ++++ > 24 files changed, 97 insertions(+) > > diff --git a/Documentation/devicetree/bindings/crypto/st,stm32-hash.yaml b/Documentation/devicetree/bindings/crypto/st,stm32-hash.yaml > index b767ec72a999..daf8dcaef627 100644 > --- a/Documentation/devicetree/bindings/crypto/st,stm32-hash.yaml > +++ b/Documentation/devicetree/bindings/crypto/st,stm32-hash.yaml > @@ -50,6 +50,10 @@ properties: > power-domains: > maxItems: 1 > > + feature-domains: > + minItems: 1 > + maxItems: 3 What are the 3 entries? Rob
Hello Rob, On 7/6/23 16:51, Rob Herring wrote: > On Wed, Jul 05, 2023 at 07:27:53PM +0200, Gatien Chevallier wrote: >> feature-domains is an optional property that allows a peripheral to >> refer to one or more feature domain controller(s). >> >> Description of this property is added to all peripheral binding files of >> the peripheral under the STM32 firewall controllers. It allows an accurate >> representation of the hardware, where various peripherals are connected >> to this firewall bus. The firewall can then check the peripheral accesses >> before allowing it to probe. >> >> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com> >> --- >> >> Disclaimer: Some error with dtbs_check will be observed as I've >> considered the property to be generic, as Rob asked >> >> Documentation/devicetree/bindings/crypto/st,stm32-hash.yaml | 4 ++++ >> Documentation/devicetree/bindings/dma/st,stm32-dma.yaml | 4 ++++ >> Documentation/devicetree/bindings/dma/st,stm32-dmamux.yaml | 4 ++++ >> Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml | 4 ++++ >> Documentation/devicetree/bindings/iio/adc/st,stm32-adc.yaml | 4 ++++ >> .../devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml | 4 ++++ >> Documentation/devicetree/bindings/iio/dac/st,stm32-dac.yaml | 4 ++++ >> .../devicetree/bindings/media/cec/st,stm32-cec.yaml | 4 ++++ >> Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml | 4 ++++ >> .../bindings/memory-controllers/st,stm32-fmc2-ebi.yaml | 4 ++++ >> Documentation/devicetree/bindings/mfd/st,stm32-lptimer.yaml | 4 ++++ >> Documentation/devicetree/bindings/mfd/st,stm32-timers.yaml | 5 +++++ >> Documentation/devicetree/bindings/mmc/arm,pl18x.yaml | 4 ++++ >> Documentation/devicetree/bindings/net/stm32-dwmac.yaml | 4 ++++ >> Documentation/devicetree/bindings/phy/phy-stm32-usbphyc.yaml | 4 ++++ >> .../devicetree/bindings/regulator/st,stm32-vrefbuf.yaml | 4 ++++ >> Documentation/devicetree/bindings/rng/st,stm32-rng.yaml | 4 ++++ >> Documentation/devicetree/bindings/serial/st,stm32-uart.yaml | 4 ++++ >> Documentation/devicetree/bindings/sound/st,stm32-i2s.yaml | 4 ++++ >> Documentation/devicetree/bindings/sound/st,stm32-sai.yaml | 4 ++++ >> .../devicetree/bindings/sound/st,stm32-spdifrx.yaml | 4 ++++ >> Documentation/devicetree/bindings/spi/st,stm32-qspi.yaml | 4 ++++ >> Documentation/devicetree/bindings/spi/st,stm32-spi.yaml | 4 ++++ >> Documentation/devicetree/bindings/usb/dwc2.yaml | 4 ++++ >> 24 files changed, 97 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/crypto/st,stm32-hash.yaml b/Documentation/devicetree/bindings/crypto/st,stm32-hash.yaml >> index b767ec72a999..daf8dcaef627 100644 >> --- a/Documentation/devicetree/bindings/crypto/st,stm32-hash.yaml >> +++ b/Documentation/devicetree/bindings/crypto/st,stm32-hash.yaml >> @@ -50,6 +50,10 @@ properties: >> power-domains: >> maxItems: 1 >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 > > What are the 3 entries? > > Rob I thought I was benefiting from the description of the pattern-property in the RIFSC YAML file. But yes anyway, it seems like it needs some description here as the dependency does not appear in this file. I picked 3 as a maxItems for our ST needs, I'll give it some more thought when coming back with something clearer. I will change that in V2, thank you for pointing that out. Best regards, Gatien
On 7/7/23 16:07, Oleksii Moisieiev wrote: > > Gatien Chevallier <gatien.chevallier@foss.st.com> writes: > >> feature-domains is an optional property that allows a peripheral to >> refer to one or more feature domain controller(s). >> >> Description of this property is added to all peripheral binding files of >> the peripheral under the STM32 firewall controllers. It allows an accurate >> representation of the hardware, where various peripherals are connected >> to this firewall bus. The firewall can then check the peripheral accesses >> before allowing it to probe. >> >> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com> >> --- >> >> Disclaimer: Some error with dtbs_check will be observed as I've >> considered the property to be generic, as Rob asked >> >> Documentation/devicetree/bindings/crypto/st,stm32-hash.yaml | 4 ++++ >> Documentation/devicetree/bindings/dma/st,stm32-dma.yaml | 4 ++++ >> Documentation/devicetree/bindings/dma/st,stm32-dmamux.yaml | 4 ++++ >> Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml | 4 ++++ >> Documentation/devicetree/bindings/iio/adc/st,stm32-adc.yaml | 4 ++++ >> .../devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml | 4 ++++ >> Documentation/devicetree/bindings/iio/dac/st,stm32-dac.yaml | 4 ++++ >> .../devicetree/bindings/media/cec/st,stm32-cec.yaml | 4 ++++ >> Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml | 4 ++++ >> .../bindings/memory-controllers/st,stm32-fmc2-ebi.yaml | 4 ++++ >> Documentation/devicetree/bindings/mfd/st,stm32-lptimer.yaml | 4 ++++ >> Documentation/devicetree/bindings/mfd/st,stm32-timers.yaml | 5 +++++ >> Documentation/devicetree/bindings/mmc/arm,pl18x.yaml | 4 ++++ >> Documentation/devicetree/bindings/net/stm32-dwmac.yaml | 4 ++++ >> Documentation/devicetree/bindings/phy/phy-stm32-usbphyc.yaml | 4 ++++ >> .../devicetree/bindings/regulator/st,stm32-vrefbuf.yaml | 4 ++++ >> Documentation/devicetree/bindings/rng/st,stm32-rng.yaml | 4 ++++ >> Documentation/devicetree/bindings/serial/st,stm32-uart.yaml | 4 ++++ >> Documentation/devicetree/bindings/sound/st,stm32-i2s.yaml | 4 ++++ >> Documentation/devicetree/bindings/sound/st,stm32-sai.yaml | 4 ++++ >> .../devicetree/bindings/sound/st,stm32-spdifrx.yaml | 4 ++++ >> Documentation/devicetree/bindings/spi/st,stm32-qspi.yaml | 4 ++++ >> Documentation/devicetree/bindings/spi/st,stm32-spi.yaml | 4 ++++ >> Documentation/devicetree/bindings/usb/dwc2.yaml | 4 ++++ >> 24 files changed, 97 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/crypto/st,stm32-hash.yaml b/Documentation/devicetree/bindings/crypto/st,stm32-hash.yaml >> index b767ec72a999..daf8dcaef627 100644 >> --- a/Documentation/devicetree/bindings/crypto/st,stm32-hash.yaml >> +++ b/Documentation/devicetree/bindings/crypto/st,stm32-hash.yaml >> @@ -50,6 +50,10 @@ properties: >> power-domains: >> maxItems: 1 >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 >> + > > I beliewe feature-domains is generic binding. This means that maxItems > can be implementation dependend. I would rather drop maxItems so the > following format will be possible: > > feature-domains = <&etzpc 1>, <&etzpc 2>, <&some_other_domain 1 2 3 4> > feature-domain-names = "firewall 1", "firewall 2", "other_domain" > I'd prefer to drop the maxItems as well. I've been told at one point in the first series to choose a number for this maybe picking a high but reasonnable number is preferrable. Based on How to Get Your DT Schema Bindings Accepted in Less than 10 Iterations pdf published by Krzysztof, I see that examples use minItems/maxItems. But I can't find if it's mandatory > Also I beliewe driver will handle feature-domain-names property so it > will parse feature-domains only related to the firewall. > Yep, in case of multiple feature-domains, it could be nice. >> required: >> - compatible >> - reg >> diff --git a/Documentation/devicetree/bindings/dma/st,stm32-dma.yaml b/Documentation/devicetree/bindings/dma/st,stm32-dma.yaml >> index 329847ef096a..2236ac95574b 100644 >> --- a/Documentation/devicetree/bindings/dma/st,stm32-dma.yaml >> +++ b/Documentation/devicetree/bindings/dma/st,stm32-dma.yaml >> @@ -82,6 +82,10 @@ properties: >> description: if defined, it indicates that the controller >> supports memory-to-memory transfer >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 >> + >> required: >> - compatible >> - reg >> diff --git a/Documentation/devicetree/bindings/dma/st,stm32-dmamux.yaml b/Documentation/devicetree/bindings/dma/st,stm32-dmamux.yaml >> index e722fbcd8a5f..47ae890f5bd9 100644 >> --- a/Documentation/devicetree/bindings/dma/st,stm32-dmamux.yaml >> +++ b/Documentation/devicetree/bindings/dma/st,stm32-dmamux.yaml >> @@ -28,6 +28,10 @@ properties: >> resets: >> maxItems: 1 >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 >> + >> required: >> - compatible >> - reg >> diff --git a/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml b/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml >> index 94b75d9f66cd..326a96741f50 100644 >> --- a/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml >> +++ b/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml >> @@ -99,6 +99,10 @@ properties: >> >> wakeup-source: true >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 >> + >> required: >> - compatible >> - reg >> diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.yaml b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.yaml >> index 995cbf8cefc6..3eb20d67f0fc 100644 >> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.yaml >> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.yaml >> @@ -93,6 +93,10 @@ properties: >> '#size-cells': >> const: 0 >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 >> + >> allOf: >> - if: >> properties: >> diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml b/Documentation/devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml >> index 1970503389aa..bc34ae172417 100644 >> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml >> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml >> @@ -59,6 +59,10 @@ properties: >> If not, SPI CLKOUT frequency will not be accurate. >> maximum: 20000000 >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 >> + >> required: >> - compatible >> - reg >> diff --git a/Documentation/devicetree/bindings/iio/dac/st,stm32-dac.yaml b/Documentation/devicetree/bindings/iio/dac/st,stm32-dac.yaml >> index 04045b932bd2..90d35a2a6504 100644 >> --- a/Documentation/devicetree/bindings/iio/dac/st,stm32-dac.yaml >> +++ b/Documentation/devicetree/bindings/iio/dac/st,stm32-dac.yaml >> @@ -45,6 +45,10 @@ properties: >> '#size-cells': >> const: 0 >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 >> + >> additionalProperties: false >> >> required: >> diff --git a/Documentation/devicetree/bindings/media/cec/st,stm32-cec.yaml b/Documentation/devicetree/bindings/media/cec/st,stm32-cec.yaml >> index 2314a9a14650..f88e3c0e6175 100644 >> --- a/Documentation/devicetree/bindings/media/cec/st,stm32-cec.yaml >> +++ b/Documentation/devicetree/bindings/media/cec/st,stm32-cec.yaml >> @@ -29,6 +29,10 @@ properties: >> - const: cec >> - const: hdmi-cec >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 >> + >> required: >> - compatible >> - reg >> diff --git a/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml b/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml >> index 6b3e413cedb2..4fa1d14910df 100644 >> --- a/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml >> +++ b/Documentation/devicetree/bindings/media/st,stm32-dcmi.yaml >> @@ -36,6 +36,10 @@ properties: >> resets: >> maxItems: 1 >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 >> + >> port: >> $ref: /schemas/graph.yaml#/$defs/port-base >> unevaluatedProperties: false >> diff --git a/Documentation/devicetree/bindings/memory-controllers/st,stm32-fmc2-ebi.yaml b/Documentation/devicetree/bindings/memory-controllers/st,stm32-fmc2-ebi.yaml >> index 14f1833d37c9..63b3d012147b 100644 >> --- a/Documentation/devicetree/bindings/memory-controllers/st,stm32-fmc2-ebi.yaml >> +++ b/Documentation/devicetree/bindings/memory-controllers/st,stm32-fmc2-ebi.yaml >> @@ -45,6 +45,10 @@ properties: >> Reflects the memory layout with four integer values per bank. Format: >> <bank-number> 0 <address of the bank> <size> >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 >> + >> patternProperties: >> "^.*@[0-4],[a-f0-9]+$": >> additionalProperties: true >> diff --git a/Documentation/devicetree/bindings/mfd/st,stm32-lptimer.yaml b/Documentation/devicetree/bindings/mfd/st,stm32-lptimer.yaml >> index 27329c5dc38e..59d770544950 100644 >> --- a/Documentation/devicetree/bindings/mfd/st,stm32-lptimer.yaml >> +++ b/Documentation/devicetree/bindings/mfd/st,stm32-lptimer.yaml >> @@ -44,6 +44,10 @@ properties: >> >> wakeup-source: true >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 >> + >> pwm: >> type: object >> additionalProperties: false >> diff --git a/Documentation/devicetree/bindings/mfd/st,stm32-timers.yaml b/Documentation/devicetree/bindings/mfd/st,stm32-timers.yaml >> index f84e09a5743b..d5ad097e94ec 100644 >> --- a/Documentation/devicetree/bindings/mfd/st,stm32-timers.yaml >> +++ b/Documentation/devicetree/bindings/mfd/st,stm32-timers.yaml >> @@ -67,6 +67,11 @@ properties: >> "#size-cells": >> const: 0 >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 >> + >> + >> pwm: >> type: object >> additionalProperties: false >> diff --git a/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml b/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml >> index 2459a55ed540..6ebedee65153 100644 >> --- a/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml >> +++ b/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml >> @@ -79,6 +79,10 @@ properties: >> - const: rx >> - const: tx >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 >> + >> power-domains: true >> >> resets: >> diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml >> index fc8c96b08d7d..0e408dc85c13 100644 >> --- a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml >> +++ b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml >> @@ -93,6 +93,10 @@ properties: >> select RCC clock instead of ETH_REF_CLK. >> type: boolean >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 >> + >> required: >> - compatible >> - clocks >> diff --git a/Documentation/devicetree/bindings/phy/phy-stm32-usbphyc.yaml b/Documentation/devicetree/bindings/phy/phy-stm32-usbphyc.yaml >> index 24a3dbde223b..b9ac20c8bf05 100644 >> --- a/Documentation/devicetree/bindings/phy/phy-stm32-usbphyc.yaml >> +++ b/Documentation/devicetree/bindings/phy/phy-stm32-usbphyc.yaml >> @@ -55,6 +55,10 @@ properties: >> description: number of clock cells for ck_usbo_48m consumer >> const: 0 >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 >> + >> # Required child nodes: >> >> patternProperties: >> diff --git a/Documentation/devicetree/bindings/regulator/st,stm32-vrefbuf.yaml b/Documentation/devicetree/bindings/regulator/st,stm32-vrefbuf.yaml >> index 05f4ad2c7d3a..02cefe4ef42b 100644 >> --- a/Documentation/devicetree/bindings/regulator/st,stm32-vrefbuf.yaml >> +++ b/Documentation/devicetree/bindings/regulator/st,stm32-vrefbuf.yaml >> @@ -30,6 +30,10 @@ properties: >> vdda-supply: >> description: phandle to the vdda input analog voltage. >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 >> + >> required: >> - compatible >> - reg >> diff --git a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml >> index 187b172d0cca..79eb5f5bd252 100644 >> --- a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml >> +++ b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml >> @@ -30,6 +30,10 @@ properties: >> type: boolean >> description: If set enable the clock detection management >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 >> + >> required: >> - compatible >> - reg >> diff --git a/Documentation/devicetree/bindings/serial/st,stm32-uart.yaml b/Documentation/devicetree/bindings/serial/st,stm32-uart.yaml >> index 1df8ffe95fc6..893978e7170f 100644 >> --- a/Documentation/devicetree/bindings/serial/st,stm32-uart.yaml >> +++ b/Documentation/devicetree/bindings/serial/st,stm32-uart.yaml >> @@ -70,6 +70,10 @@ properties: >> enum: [1, 2, 4, 8, 12, 14, 16] >> default: 8 >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 >> + >> allOf: >> - $ref: rs485.yaml# >> - $ref: serial.yaml# >> diff --git a/Documentation/devicetree/bindings/sound/st,stm32-i2s.yaml b/Documentation/devicetree/bindings/sound/st,stm32-i2s.yaml >> index b9111d375b93..64c5898e51f8 100644 >> --- a/Documentation/devicetree/bindings/sound/st,stm32-i2s.yaml >> +++ b/Documentation/devicetree/bindings/sound/st,stm32-i2s.yaml >> @@ -65,6 +65,10 @@ properties: >> $ref: audio-graph-port.yaml# >> unevaluatedProperties: false >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 >> + >> required: >> - compatible >> - "#sound-dai-cells" >> diff --git a/Documentation/devicetree/bindings/sound/st,stm32-sai.yaml b/Documentation/devicetree/bindings/sound/st,stm32-sai.yaml >> index 56d206f97a96..9bc08b7645dc 100644 >> --- a/Documentation/devicetree/bindings/sound/st,stm32-sai.yaml >> +++ b/Documentation/devicetree/bindings/sound/st,stm32-sai.yaml >> @@ -48,6 +48,10 @@ properties: >> clock-names: >> maxItems: 3 >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 >> + >> required: >> - compatible >> - reg >> diff --git a/Documentation/devicetree/bindings/sound/st,stm32-spdifrx.yaml b/Documentation/devicetree/bindings/sound/st,stm32-spdifrx.yaml >> index bc48151b9adb..f00e5db9ee3b 100644 >> --- a/Documentation/devicetree/bindings/sound/st,stm32-spdifrx.yaml >> +++ b/Documentation/devicetree/bindings/sound/st,stm32-spdifrx.yaml >> @@ -50,6 +50,10 @@ properties: >> resets: >> maxItems: 1 >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 >> + >> required: >> - compatible >> - "#sound-dai-cells" >> diff --git a/Documentation/devicetree/bindings/spi/st,stm32-qspi.yaml b/Documentation/devicetree/bindings/spi/st,stm32-qspi.yaml >> index 8bba965a9ae6..2ac136802467 100644 >> --- a/Documentation/devicetree/bindings/spi/st,stm32-qspi.yaml >> +++ b/Documentation/devicetree/bindings/spi/st,stm32-qspi.yaml >> @@ -46,6 +46,10 @@ properties: >> - const: tx >> - const: rx >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 >> + >> required: >> - compatible >> - reg >> diff --git a/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml b/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml >> index 9ca1a843c820..725c26daabe4 100644 >> --- a/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml >> +++ b/Documentation/devicetree/bindings/spi/st,stm32-spi.yaml >> @@ -59,6 +59,10 @@ properties: >> - const: rx >> - const: tx >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 >> + >> patternProperties: >> "^[a-zA-Z][a-zA-Z0-9,+\\-._]{0,63}@[0-9a-f]+$": >> type: object >> diff --git a/Documentation/devicetree/bindings/usb/dwc2.yaml b/Documentation/devicetree/bindings/usb/dwc2.yaml >> index d3506090f8b1..c372caf154fc 100644 >> --- a/Documentation/devicetree/bindings/usb/dwc2.yaml >> +++ b/Documentation/devicetree/bindings/usb/dwc2.yaml >> @@ -171,6 +171,10 @@ properties: >> >> tpl-support: true >> >> + feature-domains: >> + minItems: 1 >> + maxItems: 3 >> + >> dependencies: >> port: [ usb-role-switch ] >> role-switch-default-mode: [ usb-role-switch ] > >
Hello Krzysztof, On 7/6/23 11:29, Gatien CHEVALLIER wrote: > Hello Krzysztof, > > Firstly, I will correct the bindings error pointed by Rob's robot. > Obviously, I did not pass the bindings check the proper way or maybe I'm > running an old version. > > On 7/6/23 08:28, Krzysztof Kozlowski wrote: >> On 05/07/2023 19:27, Gatien Chevallier wrote: >>> Document RIFSC (RIF security controller). RIFSC is a firewall controller >>> composed of different kinds of hardware resources. >>> >>> Signed-off-by: Gatien Chevallier <gatien.chevallier@foss.st.com> >> >> A nit, subject: drop second/last, redundant "device tree bindings for". >> The "dt-bindings" prefix is already stating that these are bindings. 4 >> words of your 6 word subject is meaningless... > > Ack, I will rephrase, it is indeed redundant > >> >>> --- >>> .../bindings/bus/st,stm32-rifsc.yaml | 101 ++++++++++++++++++ >>> 1 file changed, 101 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/bus/st,stm32-rifsc.yaml >>> >>> diff --git >>> a/Documentation/devicetree/bindings/bus/st,stm32-rifsc.yaml >>> b/Documentation/devicetree/bindings/bus/st,stm32-rifsc.yaml >>> new file mode 100644 >>> index 000000000000..68d585ed369c >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/bus/st,stm32-rifsc.yaml >> >> Filename like compatible, unless you know list of compatibles will >> grow... but then add them. >> >>> @@ -0,0 +1,101 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/bus/st,stm32-rifsc.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: STM32 Resource isolation framework security controller bindings >> >> Drop bindings > > Ack > >> >>> + >>> +maintainers: >>> + - Gatien Chevallier <gatien.chevallier@foss.st.com> >>> + >>> +description: | >>> + Resource isolation framework (RIF) is a comprehensive set of >>> hardware blocks >>> + designed to enforce and manage isolation of STM32 hardware >>> resources like >>> + memory and peripherals. >>> + >>> + The RIFSC (RIF security controller) is composed of three sets of >>> registers, >>> + each managing a specific set of hardware resources: >>> + - RISC registers associated with RISUP logic (resource isolation >>> device unit >>> + for peripherals), assign all non-RIF aware peripherals to >>> zero, one or >>> + any security domains (secure, privilege, compartment). >>> + - RIMC registers: associated with RIMU logic (resource isolation >>> master >>> + unit), assign all non RIF-aware bus master to one security >>> domain by >>> + setting secure, privileged and compartment information on the >>> system bus. >>> + Alternatively, the RISUP logic controlling the device port >>> access to a >>> + peripheral can assign target bus attributes to this peripheral >>> master port >>> + (supported attribute: CID). >>> + - RISC registers associated with RISAL logic (resource isolation >>> device unit >>> + for address space - Lite version), assign address space >>> subregions to one >>> + security domains (secure, privilege, compartment). >>> + >>> +properties: >>> + compatible: >>> + const: st,stm32mp25-rifsc >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + "#address-cells": >>> + const: 1 >>> + >>> + "#size-cells": >>> + const: 1 >>> + >>> + "#feature-domain-cells": >>> + const: 1 >>> + >>> + ranges: true >>> + >>> + feature-domain-controller: true >>> + >>> +patternProperties: >>> + "^.*@[0-9a-f]+$": >>> + description: Peripherals >>> + type: object >>> + properties: >>> + feature-domains: >>> + minItems: 1 >>> + maxItems: 2 >>> + description: >>> + The first argument must always be a phandle that >>> references to the >>> + firewall controller of the peripheral. The second can >>> contain the >>> + platform specific firewall ID of the peripheral. >> >> It does not make much sense to me to have hierarchy parent-child and via >> phandle at the same time. You express the similar relationship twice > Thank you for pointing this out. > > About the parent-child relation: > > The bus-like device tree architecture allows a bus-probe mechanism with > which we want to check accesses of peripherals before probing their > driver. This has several advantages: > -This bus architecture provides a clearer view of the hardware. > -No peripheral driver modifications as it is fully handled by the > firewall drivers. > -Drivers for devices that aren't accessible will not even be probed => > no probe fail. > > It would be possible to manage this mechanism another way by handling > probe deferrals in drivers. But it would mean modifying every driver > with a check on ST firewall that we probe and some of them aren't from > STMicroelectronics. > > About the phandle relation: > > I agree on the fact that this double expression of the relationship is > redundant. > > I've done it this way because there will be other nodes outside the > RIFSC node that will need to reference it as their feature-domain > controller. I kept the same information in the property to be coherent > between all. > > For nodes under the RIFSC, the phandle is indeed useless and could be > removed, just to leave the firewall ID. And I'm inclined to do so. I > just have one worry on the YAML binding files where I will have a > pattern property in the RIFSC that will state something and maybe > another description in the peripheral YAML files. What is your take on > that? > Looking back at it, feature-domains is a phandle-array. I guess I can't derogate to the following architecture: items: - items: - description: A phandle - description: 1st arg cell - description: 2nd arg cell can I? Some devices' nodes that are not subnodes of the firewall controllers will need the phandle reference. Should I keep the redundant information then? Best regards, Gatien >> >>> + >>> +required: >>> + - compatible >>> + - reg >>> + - "#address-cells" >>> + - "#size-cells" >>> + - feature-domain-controller >>> + - "#feature-domain-cells" >>> + - ranges >>> + >>> +additionalProperties: false >>> + >>> +examples: >>> + - | >>> + // In this example, the usart2 device refers to rifsc as its domain >>> + // controller. >>> + // Access rights are verified before creating devices. >>> + >>> + #include <dt-bindings/interrupt-controller/arm-gic.h> >>> + >>> + rifsc: rifsc-bus@42080000 { >>> + compatible = "st,stm32mp25-rifsc"; >>> + reg = <0x42080000 0x1000>; >>> + #address-cells = <1>; >>> + #size-cells = <1>; >>> + ranges; >>> + feature-domain-controller; >>> + #feature-domain-cells = <1>; >>> + >>> + usart2: serial@400e0000 { >>> + compatible = "st,stm32h7-uart"; >>> + reg = <0x400e0000 0x400>; >>> + interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>; >>> + clocks = <&ck_flexgen_08>; >>> + feature-domains = <&rifsc 32>; >>> + status = "disabled"; >> >> No status in the examples. >> >>> + }; >>> + }; >> >> Best regards, >> Krzysztof >> > > Best regards, > Gatien
Introduce STM32 Firewall framework for STM32MP1x and STM32MP2x platforms. STM32MP1x(ETZPC) and STM32MP2x(RIFSC) Firewall controllers register to the framework to offer firewall services such as access granting. This series of patches is a new approach on the previous STM32 system bus, history is available here: https://lore.kernel.org/lkml/20230127164040.1047583/ The need for such framework arises from the fact that there are now multiple hardware firewalls implemented across multiple products. Drivers are shared between different products, using the same code. When it comes to firewalls, the purpose mostly stays the same: Protect hardware resources. But the implementation differs, and there are multiple types of firewalls: peripheral, memory, ... Some hardware firewall controllers such as the RIFSC implemented on STM32MP2x platforms may require to take ownership of a resource before being able to use it, hence the requirement for firewall services to take/release the ownership of such resources. On the other hand, hardware firewall configurations are becoming more and more complex. These mecanisms prevent platform crashes or other firewall-related incoveniences by denying access to some resources. The stm32 firewall framework offers an API that is defined in firewall controllers drivers to best fit the specificity of each firewall. For every peripherals protected by either the ETZPC or the RIFSC, the firewall framework checks the firewall controlelr registers to see if the peripheral's access is granted to the Linux kernel. If not, the peripheral is configured as secure, the node is marked populated, so that the driver is not probed for that device. The firewall framework relies on the feature-domain-controller device tree bindings: https://lore.kernel.org/lkml/0c0a82bb-18ae-d057-562b. It is used by peripherals to reference a domain controller, in this case a firewall feature domain. The bus uses the ID referenced by the feature-domains property to know where to look in the firewall to get the security configuration for the peripheral. This allows a device tree description rather than a hardcoded peripheral table in the bus driver. The STM32 ETZPC device is responsible for filtering accesses based on security level, or co-processor isolation for any resource connected to it. The RIFSC is responsible for filtering accesses based on Compartment ID / security level / privilege level for any resource connected to it. STM32MP13/15/25 SoC device tree files are updated in this series to implement this mecanism. Oleksii Moisieiev (1): dt-bindings: Document common device controller bindings Gatien Chevallier (9): dt-bindings: bus: add device tree bindings for RIFSC dt-bindings: bus: add device tree bindings for ETZPC dt-bindings: treewide: add feature-domains description in binding files firewall: introduce stm32_firewall framework bus: rifsc: introduce RIFSC firewall controller driver arm64: dts: st: add RIFSC as a domain controller for STM32MP25x boards bus: etzpc: introduce ETZPC firewall controller driver ARM: dts: stm32: add ETZPC as a system bus for STM32MP15x boards ARM: dts: stm32: add ETZPC as a system bus for STM32MP13x boards .../bindings/bus/st,stm32-etzpc.yaml | 90 + .../bindings/bus/st,stm32-rifsc.yaml | 101 + .../bindings/crypto/st,stm32-hash.yaml | 4 + .../devicetree/bindings/dma/st,stm32-dma.yaml | 4 + .../bindings/dma/st,stm32-dmamux.yaml | 4 + .../feature-domain-controller.yaml | 84 + .../devicetree/bindings/i2c/st,stm32-i2c.yaml | 4 + .../bindings/iio/adc/st,stm32-adc.yaml | 4 + .../bindings/iio/adc/st,stm32-dfsdm-adc.yaml | 4 + .../bindings/iio/dac/st,stm32-dac.yaml | 4 + .../bindings/media/cec/st,stm32-cec.yaml | 4 + .../bindings/media/st,stm32-dcmi.yaml | 4 + .../memory-controllers/st,stm32-fmc2-ebi.yaml | 4 + .../bindings/mfd/st,stm32-lptimer.yaml | 4 + .../bindings/mfd/st,stm32-timers.yaml | 5 + .../devicetree/bindings/mmc/arm,pl18x.yaml | 4 + .../devicetree/bindings/net/stm32-dwmac.yaml | 4 + .../bindings/phy/phy-stm32-usbphyc.yaml | 4 + .../bindings/regulator/st,stm32-vrefbuf.yaml | 4 + .../devicetree/bindings/rng/st,stm32-rng.yaml | 4 + .../bindings/serial/st,stm32-uart.yaml | 4 + .../bindings/sound/st,stm32-i2s.yaml | 4 + .../bindings/sound/st,stm32-sai.yaml | 4 + .../bindings/sound/st,stm32-spdifrx.yaml | 4 + .../bindings/spi/st,stm32-qspi.yaml | 4 + .../devicetree/bindings/spi/st,stm32-spi.yaml | 4 + .../devicetree/bindings/usb/dwc2.yaml | 4 + MAINTAINERS | 7 + arch/arm/boot/dts/st/stm32mp131.dtsi | 1027 +++--- arch/arm/boot/dts/st/stm32mp133.dtsi | 51 +- arch/arm/boot/dts/st/stm32mp13xc.dtsi | 19 +- arch/arm/boot/dts/st/stm32mp13xf.dtsi | 19 +- arch/arm/boot/dts/st/stm32mp151.dtsi | 2757 +++++++++-------- arch/arm/boot/dts/st/stm32mp153.dtsi | 52 +- arch/arm/boot/dts/st/stm32mp15xc.dtsi | 19 +- arch/arm64/Kconfig.platforms | 1 + arch/arm64/boot/dts/st/stm32mp251.dtsi | 5 +- drivers/bus/Kconfig | 10 + drivers/bus/Makefile | 1 + drivers/bus/stm32_etzpc.c | 137 + drivers/bus/stm32_firewall.c | 252 ++ drivers/bus/stm32_firewall.h | 83 + drivers/bus/stm32_rifsc.c | 248 ++ include/linux/bus/stm32_firewall_device.h | 134 + 44 files changed, 3276 insertions(+), 1918 deletions(-) create mode 100644 Documentation/devicetree/bindings/bus/st,stm32-etzpc.yaml create mode 100644 Documentation/devicetree/bindings/bus/st,stm32-rifsc.yaml create mode 100644 Documentation/devicetree/bindings/feature-controllers/feature-domain-controller.yaml create mode 100644 drivers/bus/stm32_etzpc.c create mode 100644 drivers/bus/stm32_firewall.c create mode 100644 drivers/bus/stm32_firewall.h create mode 100644 drivers/bus/stm32_rifsc.c create mode 100644 include/linux/bus/stm32_firewall_device.h