linux-kernel - Re: [PATCH 3/3] mailbox: sunxi-msgbox: Add a new mailbox driver

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: <20180301103259.nomjbwkw4xbvhbfv@flea>
Date:   Thu, 1 Mar 2018 11:32:59 +0100
From:   Maxime Ripard <maxime.ripard@...tlin.com>
To:     Samuel Holland <samuel@...lland.org>
Cc:     Chen-Yu Tsai <wens@...e.org>,
        Jassi Brar <jassisinghbrar@...il.com>,
        Rob Herring <robh+dt@...nel.org>, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
        Andre Przywara <andre.przywara@....com>
Subject: Re: [PATCH 3/3] mailbox: sunxi-msgbox: Add a new mailbox driver

On Wed, Feb 28, 2018 at 11:19:11AM -0600, Samuel Holland wrote:
> Hi,
> 
> On 02/28/18 02:32, Maxime Ripard wrote:
> > On Tue, Feb 27, 2018 at 08:27:14PM -0600, Samuel Holland wrote:
> >> +	/*
> >> +	 * The failure path should not disable the clock or assert the reset,
> >> +	 * because the PSCI implementation in firmware relies on this device
> >> +	 * being functional. Claiming the clock in this driver is required to
> >> +	 * prevent Linux from turning it off.
> >> +	 */
> >> +	ret = clk_prepare_enable(clk);
> >> +	if (ret) {
> >> +		dev_err(dev, "Failed to enable bus clock: %d\n", ret);
> >> +		return ret;
> >> +	}
> > 
> > You don't need it to be always on though. You only need it to be
> > enabled when you access the registers (on both sides I guess?). So you
> > could very well enable the clock in your registers accessors in Linux,
> > and do the same in the ARISC firmware. That should work.
> 
> It does need to always be on because the *PSCI* implementation (ATF) also uses
> SCPI concurrently with Linux (on a separate channel). Turning off the clock
> anywhere in Linux could turn it off in the middle of a PSCI SCPI call on a
> different CPU, causing ATF to hang in EL3 (which would be very bad).

Then the above code doesn't fix anything. You should mark the clock
critical, otherwise that clock will be turned off if the driver is
compiled as a module and not loaded (or loaded later), or if the
driver is not even compiled in.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)