linux-kernel - Re: [PATCH v10 03/19] arm: fiq: Replace default FIQ handler

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 04 Sep 2014 10:09:20 +0100
From:	Daniel Thompson <daniel.thompson@...aro.org>
To:	Russell King - ARM Linux <linux@....linux.org.uk>
CC:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	kgdb-bugreport@...ts.sourceforge.net, patches@...aro.org,
	linaro-kernel@...ts.linaro.org,
	John Stultz <john.stultz@...aro.org>,
	Anton Vorontsov <anton.vorontsov@...aro.org>,
	Colin Cross <ccross@...roid.com>, kernel-team@...roid.com,
	Rob Herring <robherring2@...il.com>,
	Linus Walleij <linus.walleij@...aro.org>,
	Ben Dooks <ben.dooks@...ethink.co.uk>,
	Catalin Marinas <catalin.marinas@....com>,
	Dave Martin <Dave.Martin@....com>,
	Fabio Estevam <festevam@...il.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Nicolas Pitre <nico@...aro.org>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH v10 03/19] arm: fiq: Replace default FIQ handler

On 03/09/14 20:34, Russell King - ARM Linux wrote:
> On Wed, Sep 03, 2014 at 11:21:30AM +0100, Daniel Thompson wrote:
>> On 02/09/14 17:42, Russell King - ARM Linux wrote:
>>> Yes, it does, because unlike the x86 community, we have a wide range
>>> of platforms, and platform code does not go through the same path or
>>> get the same review as core ARM code.
>>>
>>> As I already pointed out, with a notifier, it's very easy to sneak
>>> something into the FIQ path by submitting a patch for platform code
>>> which calls the registration function.  That's going to be pretty
>>> difficult to spot amongst the 3000+ messages on the linux-arm-kernel
>>> list each month in order to give it the review that it would need.
>>> That's especially true as I now ignore almost all most platform
>>> code patches as we have Arnd and Olof to look at that.
>>>
>>> So, unless you can come up with a proposal which ensures that there
>>> is sufficient review triggered when someone decides to call the
>>> notifier registration function...
>>
>> Reflecting upon this and upon Thomas' comments about only using FIQ for
>> watchdog, backtrace and performance monitoring...
>>
>> The short version is, "I was wrong and should have done what you said in
>> the first place".
>>
>> The long version adds, "because the coupling concerns were spurious; the
>> only proposed users of the default FIQ handler outside of core ARM code,
>> are ARM-centric irqchip drivers."
> 
> I would say that the ARM specific changes to entry-armv.S and setup.c
> are correct.  All that you're doing there is to replace the existing
> default no-op FIQ handler with some additional code which gets us into
> SVC mode and back out, but itself is also a no-op.  In other words, no
> real change.
> 
> That's a good first patch, and one which I would actually like to have
> in my tree sooner rather than later, so that I can split that out from
> my prototype code.

So would I!

I did some rebasing yesterday to put anything to do with kgdb right at
the back of the queue. This "good first patch" is now actually the first
patch; where the nofifier used to be it currently calls do_unexp_fiq()
making it very close to "no real change".

BTW do_unexp_fiq() calls printk() but

> I can also split out from it the ARM generic changes for implementing
> the FIQ dumping too, which gives us a second patch.  With a bit of
> additional work, much of that should actually be generic code, not
> ARM or x86 specific code.  That's going to annoy x86 people a little
> because some of that is being reworked...

So far I have been testing the action after patch review using the big
kgdb patches on top of it.

Today I plan to remove the kgdb stuff, drop in your version of
arch_trigger_all_cpu_backtrace() and test the results. I was going to
use the code you shared on 13 August meaning all the cpu mask bit
manipulation is in the arm-only code.

If you want me to work with something more recent then feel free to
point me at it...


> That will leave the problem of how to deal with the IRQ controller
> specifics, and how to properly wire it together with the IRQ controller
> in the loop - that is where Thomas' concerns are focused.

I'm working on those and its looking pretty good so far. This is mostly
because SGIs don't need to allocate virqs so the controversial bits of
my last patchset disappear completely.

--
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/