linux-kernel - Re: [PATCH 1/3] Input: add `SW_BOOT

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: <Yy3qj432McMJ/5rl@nixie71>
Date:   Fri, 23 Sep 2022 12:19:11 -0500
From:   Jeff LaBundy <jeff@...undy.com>
To:     Quentin Schulz <quentin.schulz@...obroma-systems.com>
Cc:     Quentin Schulz <foss+kernel@...il.net>, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, heiko@...ech.de,
        dmitry.torokhov@...il.com, klaus.goger@...obroma-systems.com,
        devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-input@...r.kernel.org
Subject: Re: [PATCH 1/3] Input: add `SW_BOOT_ALT`

Hi Quentin,

On Fri, Sep 23, 2022 at 10:25:58AM +0200, Quentin Schulz wrote:
> Hi Jeff,
> 
> On 9/22/22 19:20, Jeff LaBundy wrote:
> > Hi Quentin,
> > 
> > On Thu, Sep 22, 2022 at 12:12:09PM +0200, Quentin Schulz wrote:
> > > From: Quentin Schulz <quentin.schulz@...obroma-systems.com>
> > > 
> > > This event code represents the firmware source to use at boot.
> > > Value 0 means using "standard" firmware source, value 1 means using
> > > "alternative" firmware source.
> > > 
> > > For example, some hardware has the ability to force the BOOTROM to load
> > > the bootloader from a secondary firmware source (say SD card) instead of
> > > trying with the standard first and then the secondary. This event allows
> > > the userspace to know which firmware source was requested *in hardware*.
> > > 
> > > Signed-off-by: Quentin Schulz <quentin.schulz@...obroma-systems.com>
> > 
> > This does not seem like the right approach, especially since the switch
> > can easily be flipped after the state is already latched.
> > 
> > If the bootloader needs to pass information to the kernel (boot source or
> > otherwise), a safer and more flexible approach is to share some variables
> > in eMMC, or pass information using the kernel cmdline.
> > 
> 
> I made a terrible job at explaining what this switch is for, sorry.
> 
> Obviously, the state of the switch cannot represent which firmware boot
> source was used as only the bootloader will be able to tell (since it
> usually tries storage media in a specific order and the primary boot source
> could get corrupted at one point in time). Anyway, like you rightfully
> stated, this is useless "info" and the important one would be passed by the
> bootloader to the kernel (possibly via Device Tree fixup in case of
> Aarch64). U-Boot does this to set the memory node so this could be done
> again with a different property or something like that. Anyways, not
> something I'm really interested in.
> 
> I have a switch on my devkit which implements the BOOT_ALT#/BIOS_DISABLE#
> functionality from the Q7 standard, see section 3.1.17 Miscellaneous Signals
> in the specs:
> https://sget.org/wp-content/uploads/2018/09/Qseven-Spec_2.1.pdf
> 
> """
> BIOS_DISABLE#
> /BOOT_ALT#
> Module BIOS disable input signal. Pull low to disable
> module's on-board BIOS. Allows off-module BIOS
> implementations. This signal can also be used to disable
> standard boot firmware flash device and enable an alternative
> boot firmware source, for example a boot loader.
> """
> 
> This is basically the configuration of the firmware boot source to use for
> *the next boot*. It does not represent which boot source was used, nor which
> one will effectively be used.
> 
> In our case, this switch electrically disables eMMC and SPI flashes and only
> allow to boot from SD card (this switch is then electrically overridden by
> another GPIO at runtime by the bootloader/Linux kernel, but the state of the
> switch is still available to the user via another GPIO).

Thanks for the additional detail and the use-case is quite clear; I just
don't think input is the right home for this. Input makes more sense for
switches that a user may change during runtime with the expectation that
an event handler effects some sort of response.

Such is the case for lid open/close and headphone insertion, but here we
are just interested in the state of a muxed GPIO.

> 
> I have this switch on the board and I want to expose its state to the user,
> if this new event code is not possible/a good idea what would you suggest we
> could use?
> 
> Note that we already support the same switch but in a different way: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts#n167
> We are just configuring the GPIOs into the GPIO mode with a pull-up, and
> then it's up to the user to use gpiod or gpio-sysfs to check the state of
> the GPIO used for this switch. I don't like this, very not user-friendly and
> was looking for something better :)

Actually, that's exactly what I was going to suggest. What in particular
is not user-friendly about it?

Of course, this is just my opinion as a fellow customer of input and it
is ultimately up to Dmitry.

> 
> Hope I explained myself a bit better this time, lemme know if I can clarify
> anything.
> 
> Thanks!
> Quentin

Kind regards,
Jeff LaBundy