linux-kernel - Re: [PATCH] devicetree: zynqmp.dtsi: Add bootmode selection support

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: <07c68809-f65f-91ff-62eb-f12aa8960634@gmail.com>
Date:   Wed, 19 Feb 2020 20:23:09 +0200
From:   Vesa Jääskeläinen <dachaac@...il.com>
To:     Mike Looijmans <mike.looijmans@...ic.nl>, robh+dt@...nel.org,
        michal.simek@...inx.com, mark.rutland@....com,
        devicetree@...r.kernel.org
Cc:     m.tretter@...gutronix.de, nava.manne@...inx.com,
        rajan.vaja@...inx.com, manish.narani@...inx.com,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] devicetree: zynqmp.dtsi: Add bootmode selection support

Hi Mike,

On 19.2.2020 14.20, Mike Looijmans wrote:
> Add bootmode override support for ZynqMP devices. Allows one to select
> a boot device by running "reboot qspi32" for example. Activate config
> item CONFIG_SYSCON_REBOOT_MODE to make this work.
> 
> Signed-off-by: Mike Looijmans <mike.looijmans@...ic.nl>
> ---
>   arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 24 ++++++++++++++++++++++++
>   1 file changed, 24 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> index 26d926eb1431..4c38d77ecbba 100644
> --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> @@ -246,6 +246,30 @@
>   			};
>   		};
>   
> +		/* Clock and Reset control registers for LPD */
> +		lpd_apb: apb@...e0000 {
> +			compatible = "syscon", "simple-mfd";
> +			reg = <0x0 0xff5e0000 0x0 0x400>;
> +			reboot-mode {
> +				compatible = "syscon-reboot-mode";
> +				offset = <0x200>;
> +				mask = <0xf100>;
> +				/* Bit(8) is the "force user" bit */
> +				mode-normal = <0x0000>;
> +				mode-psjtag = <0x0100>;
> +				mode-qspi24 = <0x1100>;
> +				mode-qspi32 = <0x2100>;
> +				mode-sd0    = <0x3100>;
> +				mode-nand   = <0x4100>;
> +				mode-sd1    = <0x6100>;
> +				mode-emmc   = <0x6100>;
> +				mode-usb0   = <0x7100>;
> +				mode-pjtag0 = <0x8100>;
> +				mode-pjtag1 = <0x9100>;
> +				mode-sd1ls  = <0xe100>;

This kinda looks a bit misuse of reboot mode support.

Usually you are signal with reboot-mode that you want to do factory 
reset, enter recovery mode or such things.

Now this signaling here is telling that this is used for selecting from 
what device to boot from.

Another problem is that this now modifies all Xilinx Zynq MPSoCs which 
is kinda wrong. This behavior should really be product/board specific 
and not common for all boards -- undoing this in product/board is 
somewhat cumbersome. Now this change hijacks the "reboot <arg>" with 
this behavior which is not so nice.

Thanks,
Vesa Jääskeläinen