[v2] dt-bindings: dvfs: Add support for generic performance domains

The CLKSCREW attack [0] exposed security vulnerabilities in energy management
implementations where untrusted software had direct access to clock and
voltage hardware controls. In this attack, the malicious software was able to
place the platform into unsafe overclocked or undervolted configurations. Such
configurations then enabled the injection of predictable faults to reveal
secrets.

Many Arm-based systems used to or still use voltage regulator and clock
frameworks in the kernel. These frameworks allow callers to independently
manipulate frequency and voltage settings. Such implementations can render
systems susceptible to this form of attack.

Attacks such as CLKSCREW are now being mitigated by not having direct and
independent control of clock and voltage in the kernel and moving that
control to a trusted entity, such as the SCP firmware or secure world
firmware/software which are to perform sanity checking on the requested
performance levels, thereby preventing any attempted malicious programming.

With the advent of such an abstraction, there is a need to replace the
generic clock and regulator bindings used by such devices with a generic
performance domains bindings.

[0] https://www.usenix.org/conference/usenixsecurity17/technical-sessions/presentation/tang

Cc: Rob Herring <robh+dt@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 .../bindings/dvfs/performance-domain.yaml     | 76 +++++++++++++++++++
 1 file changed, 76 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dvfs/performance-domain.yaml

v1[1]->v2:
	- Changed to Dual License
	- Added select: true, enum for #performance-domain-cells and
	  $ref for performance-domain
	- Changed the example to use real existing compatibles instead
	  of made-up ones

[1] https://lore.kernel.org/lkml/20201105173539.1426301-1-sudeep.holla@arm.com

On 16-11-20, 18:13, Sudeep Holla wrote:
> The CLKSCREW attack [0] exposed security vulnerabilities in energy management
> implementations where untrusted software had direct access to clock and
> voltage hardware controls. In this attack, the malicious software was able to
> place the platform into unsafe overclocked or undervolted configurations. Such
> configurations then enabled the injection of predictable faults to reveal
> secrets.
> 
> Many Arm-based systems used to or still use voltage regulator and clock
> frameworks in the kernel. These frameworks allow callers to independently
> manipulate frequency and voltage settings. Such implementations can render
> systems susceptible to this form of attack.
> 
> Attacks such as CLKSCREW are now being mitigated by not having direct and
> independent control of clock and voltage in the kernel and moving that
> control to a trusted entity, such as the SCP firmware or secure world
> firmware/software which are to perform sanity checking on the requested
> performance levels, thereby preventing any attempted malicious programming.
> 
> With the advent of such an abstraction, there is a need to replace the
> generic clock and regulator bindings used by such devices with a generic
> performance domains bindings.
> 
> [0] https://www.usenix.org/conference/usenixsecurity17/technical-sessions/presentation/tang
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  .../bindings/dvfs/performance-domain.yaml     | 76 +++++++++++++++++++
>  1 file changed, 76 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dvfs/performance-domain.yaml

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

On Mon, 16 Nov 2020 18:13:56 +0000, Sudeep Holla wrote:
> The CLKSCREW attack [0] exposed security vulnerabilities in energy management
> implementations where untrusted software had direct access to clock and
> voltage hardware controls. In this attack, the malicious software was able to
> place the platform into unsafe overclocked or undervolted configurations. Such
> configurations then enabled the injection of predictable faults to reveal
> secrets.
> 
> Many Arm-based systems used to or still use voltage regulator and clock
> frameworks in the kernel. These frameworks allow callers to independently
> manipulate frequency and voltage settings. Such implementations can render
> systems susceptible to this form of attack.
> 
> Attacks such as CLKSCREW are now being mitigated by not having direct and
> independent control of clock and voltage in the kernel and moving that
> control to a trusted entity, such as the SCP firmware or secure world
> firmware/software which are to perform sanity checking on the requested
> performance levels, thereby preventing any attempted malicious programming.
> 
> With the advent of such an abstraction, there is a need to replace the
> generic clock and regulator bindings used by such devices with a generic
> performance domains bindings.
> 
> [0] https://www.usenix.org/conference/usenixsecurity17/technical-sessions/presentation/tang
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  .../bindings/dvfs/performance-domain.yaml     | 76 +++++++++++++++++++
>  1 file changed, 76 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dvfs/performance-domain.yaml
> 
> v1[1]->v2:
> 	- Changed to Dual License
> 	- Added select: true, enum for #performance-domain-cells and
> 	  $ref for performance-domain
> 	- Changed the example to use real existing compatibles instead
> 	  of made-up ones
> 
> [1] https://lore.kernel.org/lkml/20201105173539.1426301-1-sudeep.holla@arm.com
> 


My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/ata/pata-common.example.dt.yaml: /: '#performance-domain-cells' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dvfs/performance-domain.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/ata/sata-common.example.dt.yaml: /: '#performance-domain-cells' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dvfs/performance-domain.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/ata/allwinner,sun8i-r40-ahci.example.dt.yaml: /: '#performance-domain-cells' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dvfs/performance-domain.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/ata/allwinner,sun8i-r40-ahci.example.dt.yaml: example-0: '#performance-domain-cells' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dvfs/performance-domain.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/ata/allwinner,sun8i-r40-ahci.example.dt.yaml: sata@1c18000: '#performance-domain-cells' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dvfs/performance-domain.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/ata/imx-sata.example.dt.yaml: /: '#performance-domain-cells' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/dvfs/performance-domain.yaml

and on and on...

On Mon, Nov 16, 2020 at 06:13:56PM +0000, Sudeep Holla wrote:
> The CLKSCREW attack [0] exposed security vulnerabilities in energy management
> implementations where untrusted software had direct access to clock and
> voltage hardware controls. In this attack, the malicious software was able to
> place the platform into unsafe overclocked or undervolted configurations. Such
> configurations then enabled the injection of predictable faults to reveal
> secrets.
> 
> Many Arm-based systems used to or still use voltage regulator and clock
> frameworks in the kernel. These frameworks allow callers to independently
> manipulate frequency and voltage settings. Such implementations can render
> systems susceptible to this form of attack.
> 
> Attacks such as CLKSCREW are now being mitigated by not having direct and
> independent control of clock and voltage in the kernel and moving that
> control to a trusted entity, such as the SCP firmware or secure world
> firmware/software which are to perform sanity checking on the requested
> performance levels, thereby preventing any attempted malicious programming.
> 
> With the advent of such an abstraction, there is a need to replace the
> generic clock and regulator bindings used by such devices with a generic
> performance domains bindings.
> 
> [0] https://www.usenix.org/conference/usenixsecurity17/technical-sessions/presentation/tang
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  .../bindings/dvfs/performance-domain.yaml     | 76 +++++++++++++++++++
>  1 file changed, 76 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dvfs/performance-domain.yaml
> 
> v1[1]->v2:
> 	- Changed to Dual License
> 	- Added select: true, enum for #performance-domain-cells and
> 	  $ref for performance-domain
> 	- Changed the example to use real existing compatibles instead
> 	  of made-up ones
> 
> [1] https://lore.kernel.org/lkml/20201105173539.1426301-1-sudeep.holla@arm.com
> 
> diff --git a/Documentation/devicetree/bindings/dvfs/performance-domain.yaml b/Documentation/devicetree/bindings/dvfs/performance-domain.yaml
> new file mode 100644
> index 000000000000..29fb589a5192
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dvfs/performance-domain.yaml
> @@ -0,0 +1,76 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/dvfs/performance-domain.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic performance domains
> +
> +maintainers:
> +  - Sudeep Holla <sudeep.holla@arm.com>
> +
> +description: |+
> +  This binding is intended for performance management of groups of devices or
> +  CPUs that run in the same performance domain. Performance domains must not
> +  be confused with power domains. A performance domain is defined by a set
> +  of devices that always have to run at the same performance level. For a given
> +  performance domain, there is a single point of control that affects all the
> +  devices in the domain, making it impossible to set the performance level of
> +  an individual device in the domain independently from other devices in
> +  that domain. For example, a set of CPUs that share a voltage domain, and
> +  have a common frequency control, is said to be in the same performance
> +  domain.
> +
> +  This device tree binding can be used to bind performance domain consumer
> +  devices with their performance domains provided by performance domain
> +  providers. A performance domain provider can be represented by any node in
> +  the device tree and can provide one or more performance domains. A consumer
> +  node can refer to the provider by a phandle and a set of phandle arguments
> +  (so called performance domain specifiers) of length specified by the
> +  \#performance-domain-cells property in the performance domain provider node.
> +
> +select: true

So apply to every node and...

> +
> +properties:
> +  "#performance-domain-cells":
> +    description:
> +      Number of cells in a performance domain specifier. Typically 0 for nodes
> +      representing a single performance domain and 1 for nodes providing
> +      multiple performance domains (e.g. performance controllers), but can be
> +      any value as specified by device tree binding documentation of particular
> +      provider.
> +    enum: [ 0, 1 ]
> +
> +  performance-domains:
> +    $ref: '/schemas/types.yaml#/definitions/phandle-array'
> +    description:
> +      A phandle and performance domain specifier as defined by bindings of the
> +      performance controller/provider specified by phandle.
> +
> +required:
> +  - "#performance-domain-cells"

Every node must have this!

It can only be required in actual users.

> +
> +additionalProperties: true
> +
> +examples:
> +  - |
> +    performance: performance-controller@12340000 {
> +        compatible = "qcom,cpufreq-hw";
> +        reg = <0x12340000 0x1000>;
> +        #performance-domain-cells = <1>;
> +    };
> +
> +    // The node above defines a performance controller that is a performance
> +    // domain provider and expects one cell as its phandle argument.
> +    cpus {
> +        #address-cells = <2>;
> +        #size-cells = <0>;
> +
> +        cpu@0 {
> +            device_type = "cpu";
> +            compatible = "arm,cortex-a57";
> +            reg = <0x0 0x0>;
> +            performance-domains = <&performance 1>;

Looks like the cpu schema needs an addition.

> +        };
> +    };
> +
> -- 
> 2.25.1
>

On Wed, Nov 18, 2020 at 03:20:09PM -0600, Rob Herring wrote:
> On Mon, Nov 16, 2020 at 06:13:56PM +0000, Sudeep Holla wrote:
> > The CLKSCREW attack [0] exposed security vulnerabilities in energy management
> > implementations where untrusted software had direct access to clock and
> > voltage hardware controls. In this attack, the malicious software was able to
> > place the platform into unsafe overclocked or undervolted configurations. Such
> > configurations then enabled the injection of predictable faults to reveal
> > secrets.
> > 
> > Many Arm-based systems used to or still use voltage regulator and clock
> > frameworks in the kernel. These frameworks allow callers to independently
> > manipulate frequency and voltage settings. Such implementations can render
> > systems susceptible to this form of attack.
> > 
> > Attacks such as CLKSCREW are now being mitigated by not having direct and
> > independent control of clock and voltage in the kernel and moving that
> > control to a trusted entity, such as the SCP firmware or secure world
> > firmware/software which are to perform sanity checking on the requested
> > performance levels, thereby preventing any attempted malicious programming.
> > 
> > With the advent of such an abstraction, there is a need to replace the
> > generic clock and regulator bindings used by such devices with a generic
> > performance domains bindings.
> > 
> > [0] https://www.usenix.org/conference/usenixsecurity17/technical-sessions/presentation/tang
> > 
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > ---
> >  .../bindings/dvfs/performance-domain.yaml     | 76 +++++++++++++++++++
> >  1 file changed, 76 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/dvfs/performance-domain.yaml
> > 
> > v1[1]->v2:
> > 	- Changed to Dual License
> > 	- Added select: true, enum for #performance-domain-cells and
> > 	  $ref for performance-domain
> > 	- Changed the example to use real existing compatibles instead
> > 	  of made-up ones
> > 
> > [1] https://lore.kernel.org/lkml/20201105173539.1426301-1-sudeep.holla@arm.com
> > 
> > diff --git a/Documentation/devicetree/bindings/dvfs/performance-domain.yaml b/Documentation/devicetree/bindings/dvfs/performance-domain.yaml
> > new file mode 100644
> > index 000000000000..29fb589a5192
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/dvfs/performance-domain.yaml
> > @@ -0,0 +1,76 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/dvfs/performance-domain.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Generic performance domains
> > +
> > +maintainers:
> > +  - Sudeep Holla <sudeep.holla@arm.com>
> > +
> > +description: |+
> > +  This binding is intended for performance management of groups of devices or
> > +  CPUs that run in the same performance domain. Performance domains must not
> > +  be confused with power domains. A performance domain is defined by a set
> > +  of devices that always have to run at the same performance level. For a given
> > +  performance domain, there is a single point of control that affects all the
> > +  devices in the domain, making it impossible to set the performance level of
> > +  an individual device in the domain independently from other devices in
> > +  that domain. For example, a set of CPUs that share a voltage domain, and
> > +  have a common frequency control, is said to be in the same performance
> > +  domain.
> > +
> > +  This device tree binding can be used to bind performance domain consumer
> > +  devices with their performance domains provided by performance domain
> > +  providers. A performance domain provider can be represented by any node in
> > +  the device tree and can provide one or more performance domains. A consumer
> > +  node can refer to the provider by a phandle and a set of phandle arguments
> > +  (so called performance domain specifiers) of length specified by the
> > +  \#performance-domain-cells property in the performance domain provider node.
> > +
> > +select: true
> 
> So apply to every node and...
>

New to yaml, still figuring out 😄.
From the bot build error, I now realise that I can't take shortcut to build:

$ make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/arm/dvfs/performance-domain.yaml

[...]

> > +
> > +additionalProperties: true
> > +
> > +examples:
> > +  - |
> > +    performance: performance-controller@12340000 {
> > +        compatible = "qcom,cpufreq-hw";
> > +        reg = <0x12340000 0x1000>;
> > +        #performance-domain-cells = <1>;
> > +    };
> > +
> > +    // The node above defines a performance controller that is a performance
> > +    // domain provider and expects one cell as its phandle argument.
> > +    cpus {
> > +        #address-cells = <2>;
> > +        #size-cells = <0>;
> > +
> > +        cpu@0 {
> > +            device_type = "cpu";
> > +            compatible = "arm,cortex-a57";
> > +            reg = <0x0 0x0>;
> > +            performance-domains = <&performance 1>;
>
> Looks like the cpu schema needs an addition.
>

OK.

--
Regards,
Sudeep

On Wed, Nov 18, 2020 at 3:20 PM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Nov 16, 2020 at 06:13:56PM +0000, Sudeep Holla wrote:
> > The CLKSCREW attack [0] exposed security vulnerabilities in energy management
> > implementations where untrusted software had direct access to clock and
> > voltage hardware controls. In this attack, the malicious software was able to
> > place the platform into unsafe overclocked or undervolted configurations. Such
> > configurations then enabled the injection of predictable faults to reveal
> > secrets.
> >
> > Many Arm-based systems used to or still use voltage regulator and clock
> > frameworks in the kernel. These frameworks allow callers to independently
> > manipulate frequency and voltage settings. Such implementations can render
> > systems susceptible to this form of attack.
> >
> > Attacks such as CLKSCREW are now being mitigated by not having direct and
> > independent control of clock and voltage in the kernel and moving that
> > control to a trusted entity, such as the SCP firmware or secure world
> > firmware/software which are to perform sanity checking on the requested
> > performance levels, thereby preventing any attempted malicious programming.
> >
> > With the advent of such an abstraction, there is a need to replace the
> > generic clock and regulator bindings used by such devices with a generic
> > performance domains bindings.
> >
> > [0] https://www.usenix.org/conference/usenixsecurity17/technical-sessions/presentation/tang
> >
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > ---
> >  .../bindings/dvfs/performance-domain.yaml     | 76 +++++++++++++++++++
> >  1 file changed, 76 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/dvfs/performance-domain.yaml
> >
> > v1[1]->v2:
> >       - Changed to Dual License
> >       - Added select: true, enum for #performance-domain-cells and
> >         $ref for performance-domain
> >       - Changed the example to use real existing compatibles instead
> >         of made-up ones
> >
> > [1] https://lore.kernel.org/lkml/20201105173539.1426301-1-sudeep.holla@arm.com
> >
> > diff --git a/Documentation/devicetree/bindings/dvfs/performance-domain.yaml b/Documentation/devicetree/bindings/dvfs/performance-domain.yaml
> > new file mode 100644
> > index 000000000000..29fb589a5192
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/dvfs/performance-domain.yaml
> > @@ -0,0 +1,76 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/dvfs/performance-domain.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Generic performance domains
> > +
> > +maintainers:
> > +  - Sudeep Holla <sudeep.holla@arm.com>
> > +
> > +description: |+
> > +  This binding is intended for performance management of groups of devices or
> > +  CPUs that run in the same performance domain. Performance domains must not
> > +  be confused with power domains. A performance domain is defined by a set
> > +  of devices that always have to run at the same performance level. For a given
> > +  performance domain, there is a single point of control that affects all the
> > +  devices in the domain, making it impossible to set the performance level of
> > +  an individual device in the domain independently from other devices in
> > +  that domain. For example, a set of CPUs that share a voltage domain, and
> > +  have a common frequency control, is said to be in the same performance
> > +  domain.
> > +
> > +  This device tree binding can be used to bind performance domain consumer
> > +  devices with their performance domains provided by performance domain
> > +  providers. A performance domain provider can be represented by any node in
> > +  the device tree and can provide one or more performance domains. A consumer
> > +  node can refer to the provider by a phandle and a set of phandle arguments
> > +  (so called performance domain specifiers) of length specified by the
> > +  \#performance-domain-cells property in the performance domain provider node.
> > +
> > +select: true
>
> So apply to every node and...
>
> > +
> > +properties:
> > +  "#performance-domain-cells":
> > +    description:
> > +      Number of cells in a performance domain specifier. Typically 0 for nodes
> > +      representing a single performance domain and 1 for nodes providing
> > +      multiple performance domains (e.g. performance controllers), but can be
> > +      any value as specified by device tree binding documentation of particular
> > +      provider.
> > +    enum: [ 0, 1 ]
> > +
> > +  performance-domains:
> > +    $ref: '/schemas/types.yaml#/definitions/phandle-array'
> > +    description:
> > +      A phandle and performance domain specifier as defined by bindings of the
> > +      performance controller/provider specified by phandle.
> > +
> > +required:
> > +  - "#performance-domain-cells"
>
> Every node must have this!
>
> It can only be required in actual users.

If I wasn't clear, you need to drop 'required' as 'select: true' means
apply the schema to every node.

Rob