linux-kernel - Re: [PATCH 5/8] DT/arm,gic-v3: add save-suspend-state property

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <86efmsg1gx.wl-marc.zyngier@arm.com>
Date:   Sun, 14 Jan 2018 10:48:46 +0000
From:   Marc Zyngier <marc.zyngier@....com>
To:     Derek Basehore <dbasehore@...omium.org>
Cc:     linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
        rafael.j.wysocki@...el.com, tglx@...utronix.de,
        briannorris@...omium.org
Subject: Re: [PATCH 5/8] DT/arm,gic-v3: add save-suspend-state property

On Fri, 12 Jan 2018 21:24:19 +0000,
Derek Basehore wrote:
> 
> This adds documentation for the new save-suspend-state property. This
> property enables saving and restoring the GIC for when it loses state
> in system suspend.
> 
> Signed-off-by: Derek Basehore <dbasehore@...omium.org>

Please CC the DT maintainers for all DT binding changes.

> ---
>  .../bindings/interrupt-controller/arm,gic-v3.txt   | 36 ++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt
> index 0a57f2f4167d..820556a8ffd2 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt
> +++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt
> @@ -57,6 +57,10 @@ Optional
>    occupied by the redistributors. Required if more than one such
>    region is present.
>  
> +- save-suspend-state : Bool property. Setting this has the kernel save
> +  and restore the GIC state for suspend and resume respectively. For
> +  when the GIC loses power during suspend.

The name is pretty backward. You're supposed to describe a property of
the system, not instruct what SW should do.

What about something like "powered-off-on-suspend"?

Also, does this apply to the GIC? The ITS? The whole thing? Does it
make sense to treat them independently? (hint: probably).

> +
>  Sub-nodes:
>  
>  PPI affinity can be expressed as a single "ppi-partitions" node,
> @@ -107,6 +111,38 @@ Examples:
>  		};
>  	};
>  
> +	gic: interrupt-controller@...00000 {
> +		compatible = "arm,gic-v3";
> +		#interrupt-cells = <4>;
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +		interrupt-controller;
> +		save-suspend-state;
> +
> +		reg = <0x0 0xfee00000 0 0x10000>, /* GICD */
> +		      <0x0 0xfef00000 0 0xc0000>, /* GICR */
> +		      <0x0 0xfff00000 0 0x10000>, /* GICC */
> +		      <0x0 0xfff10000 0 0x10000>, /* GICH */
> +		      <0x0 0xfff20000 0 0x10000>; /* GICV */
> +		interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH 0>;
> +		its: interrupt-controller@...20000 {
> +			compatible = "arm,gic-v3-its";
> +			msi-controller;
> +			reg = <0x0 0xfee20000 0x0 0x20000>;
> +		};
> +
> +		ppi-partitions {
> +			ppi_cluster0: interrupt-partition-0 {
> +				affinity = <&cpu_l0 &cpu_l1 &cpu_l2 &cpu_l3>;
> +			};
> +
> +			ppi_cluster1: interrupt-partition-1 {
> +				affinity = <&cpu_b0 &cpu_b1>;
> +			};
> +		};
> +	};
> +

I don't think we need another full example of the binding.

>  	gic: interrupt-controller@...10000 {
>  		compatible = "arm,gic-v3";
>  		#interrupt-cells = <4>;
> -- 
> 2.16.0.rc1.238.g530d649a79-goog
> 

Thanks,

	M.