linux-kernel - Re: [PATCH] dt-bindings: Add an enable method to RISC-V

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: <20171121110451.qm5cy5s4audfvwu5@lakrids.cambridge.arm.com>
Date:   Tue, 21 Nov 2017 11:04:52 +0000
From:   Mark Rutland <mark.rutland@....com>
To:     Palmer Dabbelt <palmer@...ive.com>
Cc:     robh+dt@...nel.org, devicetree@...r.kernel.org,
        patches@...ups.riscv.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] dt-bindings: Add an enable method to RISC-V

Hi Palmer,

On Mon, Nov 20, 2017 at 11:50:22AM -0800, Palmer Dabbelt wrote:
> RISC-V doesn't currently specify a mechanism for enabling or disabling
> CPUs.  Instead, we assume that all CPUs are enabled on boot, and if
> someone wants to save power we instead put a CPU to sleep via a WFI
> loop.
> 
> This patch adds "enable-method" to the RISC-V CPU binding, which
> currently only has the value "none".  This allows us to change the
> enable method in the future.

I think you might want to be a bit more explicit about what this means,
and this could do with a better name, as "none" sounds like the CPU is
unusable, rather than it having been placed within the kernel already by
the FW/bootloader (which IIUC is what happens currently).

As previosuly commented, I also really think you'll want to define a
simple boot protocol (like PPC spin-table) whereby the kernel can bring
each CPU into the kernel independently. That will save you a lot of pain
in future with things like kexec, suspend/resume, etc.

For arm64 we had a spin-table clone (implemented in our boot-wrapper
firmware) that allowed us to bring CPUs into the kernel explicitly.
However, we made the mistake of allowing CPUs to share a mailbox, and we
couldn't tell how many CPUs were stuck in the kernel at any point in
time (rendering kexec, suspend, etc impossible).

Thanks,
Mark.

> CC: Mark Rutland <mark.rutland@....com>
> Signed-off-by: Palmer Dabbelt <palmer@...ive.com>
> ---
>  Documentation/devicetree/bindings/riscv/cpus.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/riscv/cpus.txt b/Documentation/devicetree/bindings/riscv/cpus.txt
> index adf7b7af5dc3..dd9e1ae197e2 100644
> --- a/Documentation/devicetree/bindings/riscv/cpus.txt
> +++ b/Documentation/devicetree/bindings/riscv/cpus.txt
> @@ -82,6 +82,11 @@ described below.
>                  Value type: <string>
>                  Definition: Contains the RISC-V ISA string of this hart.  These
>                              ISA strings are defined by the RISC-V ISA manual.
> +        - cpu-enable-method:
> +		Usage: required
> +		Value type: <stringlist>
> +		Definition: Must be one of
> +			"none": This CPU's state cannot be changed.
>  
>  Example: SiFive Freedom U540G Development Kit
>  ---------------------------------------------
> @@ -105,6 +110,7 @@ Linux is allowed to run on.
>                          reg = <0>;
>                          riscv,isa = "rv64imac";
>                          status = "disabled";
> +                        enable-method = "none";
>                          L10: interrupt-controller {
>                                  #interrupt-cells = <1>;
>                                  compatible = "riscv,cpu-intc";
> @@ -130,6 +136,7 @@ Linux is allowed to run on.
>                          reg = <1>;
>                          riscv,isa = "rv64imafdc";
>                          status = "okay";
> +                        enable-method = "none";
>                          tlb-split;
>                          L13: interrupt-controller {
>                                  #interrupt-cells = <1>;
> -- 
> 2.13.6
>