linux-kernel - Re: [PATCH v3 3/5] arm64: dts: Add support for Spreadtrum SC9836 SoC in dts and Makefile

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: <20141128144650.GK25883@leverpostej>
Date:	Fri, 28 Nov 2014 14:46:51 +0000
From:	Mark Rutland <mark.rutland@....com>
To:	Will Deacon <will.deacon@....com>
Cc:	Catalin Marinas <Catalin.Marinas@....com>,
	"andrew@...n.ch" <andrew@...n.ch>,
	"heiko@...ech.de" <heiko@...ech.de>,
	"gnomes@...rguk.ukuu.org.uk" <gnomes@...rguk.ukuu.org.uk>,
	Chunyan Zhang <chunyan.zhang@...eadtrum.com>,
	"jslaby@...e.cz" <jslaby@...e.cz>,
	"jason@...edaemon.net" <jason@...edaemon.net>,
	"lanqing.liu@...eadtrum.com" <lanqing.liu@...eadtrum.com>,
	Pawel Moll <Pawel.Moll@....com>,
	"corbet@....net" <corbet@....net>,
	"zhang.lyra@...il.com" <zhang.lyra@...il.com>,
	"zhizhou.zhang@...eadtrum.com" <zhizhou.zhang@...eadtrum.com>,
	"geng.ren@...eadtrum.com" <geng.ren@...eadtrum.com>,
	"m-karicheri2@...com" <m-karicheri2@...com>,
	"shawn.guo@...escale.com" <shawn.guo@...escale.com>,
	"linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
	"grant.likely@...aro.org" <grant.likely@...aro.org>,
	"orsonzhai@...il.com" <orsonzhai@...il.com>,
	"florian.vaussard@...l.ch" <florian.vaussard@...l.ch>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"artagnon@...il.com" <artagnon@...il.com>,
	"arnd@...db.de" <arnd@...db.de>,
	"ijc+devicetree@...lion.org.uk" <ijc+devicetree@...lion.org.uk>,
	Marc Zyngier <Marc.Zyngier@....com>,
	"galak@...eaurora.org" <galak@...eaurora.org>,
	"hytszk@...il.com" <hytszk@...il.com>,
	"rrichter@...ium.com" <rrichter@...ium.com>,
	"broonie@...nel.org" <broonie@...nel.org>,
	"wei.qiao@...eadtrum.com" <wei.qiao@...eadtrum.com>,
	"sprdlinux@...elists.org" <sprdlinux@...elists.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-api@...r.kernel.org" <linux-api@...r.kernel.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"robh+dt@...nel.org" <robh+dt@...nel.org>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"broonie@...aro.org" <broonie@...aro.org>
Subject: Re: [PATCH v3 3/5] arm64: dts: Add support for Spreadtrum SC9836 SoC
 in dts and Makefile

On Fri, Nov 28, 2014 at 02:44:12PM +0000, Will Deacon wrote:
> On Fri, Nov 28, 2014 at 02:35:32PM +0000, Mark Rutland wrote:
> > On Fri, Nov 28, 2014 at 02:29:13PM +0000, Catalin Marinas wrote:
> > > On Thu, Nov 27, 2014 at 01:43:09PM +0000, Mark Rutland wrote:
> > > > On Thu, Nov 27, 2014 at 12:12:15PM +0000, Catalin Marinas wrote:
> > > > > On Thu, Nov 27, 2014 at 11:50:43AM +0000, Mark Rutland wrote:
> > > > > > On Tue, Nov 25, 2014 at 12:16:56PM +0000, Chunyan Zhang wrote:
> > > > > > > +
> > > > > > > +       timer {
> > > > > > > +               compatible = "arm,armv8-timer";
> > > > > > > +               interrupts = <1 13 0xff01>,
> > > > > > > +                            <1 14 0xff01>,
> > > > > > > +                            <1 11 0xff01>,
> > > > > > > +                            <1 10 0xff01>;
> > > > > > > +               clock-frequency = <26000000>;
> > > > > > 
> > > > > > Please remove the clock-frequency property. Your FW should initialise
> > > > > > CNTFRQ_EL0 on all CPUs (certainly PSCI 0.2 requires that you do this).
> > > > > 
> > > > > Since this comes up regularly, I think we need a dev_warn() in the arch
> > > > > timer driver when CONFIG_ARM64.
> > > > 
> > > > I'll ack such a patch ;)
> > > 
> > > How rude would this be?
> > > 
> > > diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> > > index 2133f9d59d06..aaaf3433ccb9 100644
> > > --- a/drivers/clocksource/arm_arch_timer.c
> > > +++ b/drivers/clocksource/arm_arch_timer.c
> > > @@ -371,7 +371,8 @@ arch_timer_detect_rate(void __iomem *cntbase, struct device_node *np)
> > > 		return;
> > >  
> > > 	/* Try to determine the frequency from the device tree or CNTFRQ */
> > > -	if (of_property_read_u32(np, "clock-frequency", &arch_timer_rate)) {
> > > +	if (IS_ENABLED(CONFIG_ARM64) ||
> > > +	    of_property_read_u32(np, "clock-frequency", &arch_timer_rate)) {
> > > 		if (cntbase)
> > > 			arch_timer_rate = readl_relaxed(cntbase + CNTFRQ);
> > > 		else
> > >
> > 
> > Probably too rude, given it doesn't WARN() the user.
> 
> We override broken hardware ID registers all the time in device-tree without
> dumping stack. Why is this any different?

Exposure to guests via KVM, and the fact that it's possible to write to
the register from EL3. This isn't so much broken HW (which cannot be
fixed) as broken FW (which can be fixed).

Printing the warning gives people the chance to realise and fix the
issue during bringup.

Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/