diff mbox

[3.18-rc3,v9,5/5] arm: smp: Handle ipi_cpu_backtrace() using FIQ (if available)

Message ID 1633306.naE1qIcAOt@dabox (mailing list archive)
State New, archived
Headers show

Commit Message

Tim Sander Dec. 1, 2014, 10:32 a.m. UTC
Hi Russel, Daniel

Am Freitag, 28. November 2014, 10:08:28 schrieb Russell King - ARM Linux:
> On Fri, Nov 28, 2014 at 10:10:04AM +0100, Tim Sander wrote:
> > Hi Daniel, Russell
> > 
> > Am Mittwoch, 26. November 2014, 16:17:06 schrieb Daniel Thompson:
> > > On 26/11/14 13:12, Russell King - ARM Linux wrote:
> > > > On Wed, Nov 26, 2014 at 01:46:52PM +0100, Tim Sander wrote:
> > > >> Hi Daniel
> > > >> 
> > > >> Am Dienstag, 25. November 2014, 17:26:41 schrieb Daniel Thompson:
> > > >>> Previous changes have introduced both a replacement default FIQ
> > > >>> handler
> > > >>> and an implementation of arch_trigger_all_cpu_backtrace for ARM but
> > > >>> these are currently independent of each other.
> > > >>> 
> > > >>> This patch plumbs together these features making it possible, on
> > > >>> platforms
> > > >>> that support it, to trigger backtrace using FIQ.
> > > >> 
> > > >> Does this ipi handler interfere in any way with set_fiq_handler?
> > > >> 
> > > >> As far as i know there is only one FIQ handler vector so i guess
> > > >> there is
> > > >> a
> > > >> potential conflict. But i have not worked with IPI's so i might be
> > > >> completley wrong.
> > > > 
> > > > First, the code in arch/arm/kernel/fiq.c should work with this new FIQ
> > > > code in that the new FIQ code is used as the "default" handler (as
> > > > opposed to the original handler which was a total no-op.)
> > > > 
> > > > Secondly, use of arch/arm/kernel/fiq.c in a SMP system is really not a
> > > > good idea: the FIQ registers are private to each CPU in the system,
> > > > and
> > > > there is no infrastructure to allow fiq.c to ensure that it loads the
> > > > right CPU with the register information for the provided handler.
> > 
> > Well given the races in the GIC v1. i have seen in the chips on my desk
> > initializing with for_each_possible_cpu(cpu) work_on_cpu(cpu,..) is rather
> > easy.
> > 
> > > > So, use of arch/arm/kernel/fiq.c and the IPI's use of FIQ /should/ be
> > > > mutually exclusive.
> > 
> > Yes but i digress on the assessment that this a decision between SMP and
> > non- SMP usage or the availbility of the GIC.
> 
> The two things are mutually exclusive.  You can either have FIQ being used
> for debug purposes, where we decode the FIQ reason and call some function
> (which means that we will only service one FIQ at a time) or you can use
> it in exclusive mode (provided by fiq.c) where your handler has sole usage
> of the vector, and benefits from fast and immediate servicing of the event.
As far as i am aware, die CONFIG_FIQ symbol is not pulled by all ARM 
platforms. Since there are ARM platforms which don't use this symbol but the 
hardware is fully capable of handling FIQ requests i would expect, that adding
CONFIG_FIQ to a plattform, that this platform honors the set_fiq_handler 
functionality. 
> You can't have fast and immediate servicing of the event _and_ debug usage
> at the same time.
> 
> > Well i am not against these features as they assumably improve the
> > backtrace, but it would be nice to have a config option which switches
> > between set_fiq_handler usage and the other conflicting usages of the
> > fiq.
> You have a config option already.  CONFIG_FIQ.
Yes, but if the FIQ handler is also used for IPI, set_fiq_handler gets IPI 
interrupts (with the patch starting this thread)? So i think that the patch 
needs to look like:
set_fiq_handler ?

Best regards
Tim

Comments

Russell King - ARM Linux Dec. 1, 2014, 10:38 a.m. UTC | #1
On Mon, Dec 01, 2014 at 11:32:00AM +0100, Tim Sander wrote:
> Hi Russel, Daniel
> 
> Am Freitag, 28. November 2014, 10:08:28 schrieb Russell King - ARM Linux:
> > The two things are mutually exclusive.  You can either have FIQ being used
> > for debug purposes, where we decode the FIQ reason and call some function
> > (which means that we will only service one FIQ at a time) or you can use
> > it in exclusive mode (provided by fiq.c) where your handler has sole usage
> > of the vector, and benefits from fast and immediate servicing of the event.
>
> As far as i am aware, die CONFIG_FIQ symbol is not pulled by all ARM 
> platforms. Since there are ARM platforms which don't use this symbol but the 
> hardware is fully capable of handling FIQ requests i would expect, that adding
> CONFIG_FIQ to a plattform, that this platform honors the set_fiq_handler 
> functionality. 

That whole paragraph doesn't make much sense to me.

Look, in my mind it is very simple.  If you are using CONFIG_FIQ on a
SMP platform, your life will be very difficult.  The FIQ code enabled
by that symbol is not designed to be used on SMP systems, *period*.

If you decide to enable CONFIG_FIQ, and you use that code on a SMP
platform, I'm going to say right now so it's totally clear: if you
encounter a problem, I don't want to know about it.  The code is not
designed for use on that situation.

Therefore, as far as I'm concerned, the two facilities are mututally
exclusive.

I had thought about whether the IPI FIQ should be disabled when a
replacement FIQ handler is installed, I deem it not to be a use case
that the mainline kernel needs to be concerned about.

> Yes, but if the FIQ handler is also used for IPI, set_fiq_handler gets IPI 
> interrupts (with the patch starting this thread)? So i think that the patch 
> needs to look like:
> --- a/arch/arm/kernel/traps.c
> +++ b/arch/arm/kernel/traps.c
> @@ -483,6 +483,9 @@ asmlinkage void __exception_irq_entry 
> handle_fiq_as_nmi(struct pt_regs *regs)
> +#ifndef CONFIG_FIQ
>  #ifdef CONFIG_ARM_GIC
>         gic_handle_fiq_ipi();
>  #endif
> +#endif

No.  With a single zImage kernel, you could very well have SMP and FIQ
both enabled, but have a non-SMP platform using FIQ, but also support
SMP platforms as well.  Your change prevents that happening.
Tim Sander Dec. 1, 2014, 1:54 p.m. UTC | #2
Hi Russell

Am Montag, 1. Dezember 2014, 10:38:32 schrieb Russell King - ARM Linux:
> On Mon, Dec 01, 2014 at 11:32:00AM +0100, Tim Sander wrote:
> > Hi Russel, Daniel
> > 
> > Am Freitag, 28. November 2014, 10:08:28 schrieb Russell King - ARM Linux:
> > > The two things are mutually exclusive.  You can either have FIQ being
> > > used
> > > for debug purposes, where we decode the FIQ reason and call some
> > > function
> > > (which means that we will only service one FIQ at a time) or you can use
> > > it in exclusive mode (provided by fiq.c) where your handler has sole
> > > usage
> > > of the vector, and benefits from fast and immediate servicing of the
> > > event.
> > 
> > As far as i am aware, die CONFIG_FIQ symbol is not pulled by all ARM
> > platforms. Since there are ARM platforms which don't use this symbol but
> > the hardware is fully capable of handling FIQ requests i would expect,
> > that adding CONFIG_FIQ to a plattform, that this platform honors the
> > set_fiq_handler functionality.
> 
> That whole paragraph doesn't make much sense to me.
> 
> Look, in my mind it is very simple.  If you are using CONFIG_FIQ on a
> SMP platform, your life will be very difficult.  The FIQ code enabled
> by that symbol is not designed to be used on SMP systems, *period*.
Well the only extra thing you had to do is set up the FIQ registers on every 
cpu, but i would not call that very difficult. Other than that i am not aware of 
any problems that are not also present on a uniprocessor system. So i have a 
hard time following your reasoning why SMP is different from UP in regard to 
the CONFIG_FIQ.

> If you decide to enable CONFIG_FIQ, and you use that code on a SMP
> platform, I'm going to say right now so it's totally clear: if you
> encounter a problem, I don't want to know about it.  The code is not
> designed for use on that situation.
Even with using the FIQ on a Linux SMP system you have not heard from me 
before, as i knew that this is not your problem (and that is not to say that 
there where none!). The only interface Linux has been making available is 
set_fiq_handler. So it was clear that the FIQ is its own domain otherwise 
untouched by the kernel. Now the line gets blurried with the linux kernel 
moving to use the FIQ. And with the descicions forthcoming its not only 
grabbing land it also claims a previous public path for its own. So it doesn't 
help that its planting some flowers along the way. So please be nice to the 
natural inhabitants...

And i really don't get it, that neither ARM nor the kernel community sees fast 
interrupts as a worthwhile usecase. Unfortunatly the interrupt latencies with 
Linux are at least a order of magnitude higher than the pure hardware even 
with longer pipelines can deliver. 

> Therefore, as far as I'm concerned, the two facilities are mututally
> exclusive.
Well can't have the cake and eat it too.

> I had thought about whether the IPI FIQ should be disabled when a
> replacement FIQ handler is installed, I deem it not to be a use case
> that the mainline kernel needs to be concerned about.
That would be nice.

> > Yes, but if the FIQ handler is also used for IPI, set_fiq_handler gets IPI
> > interrupts (with the patch starting this thread)? So i think that the
> > patch
> > needs to look like:
> > --- a/arch/arm/kernel/traps.c
> > +++ b/arch/arm/kernel/traps.c
> > @@ -483,6 +483,9 @@ asmlinkage void __exception_irq_entry
> > handle_fiq_as_nmi(struct pt_regs *regs)
> > +#ifndef CONFIG_FIQ
> > 
> >  #ifdef CONFIG_ARM_GIC
> >  
> >         gic_handle_fiq_ipi();
> >  
> >  #endif
> > 
> > +#endif
> 
> No.  With a single zImage kernel, you could very well have SMP and FIQ
> both enabled, but have a non-SMP platform using FIQ, but also support
> SMP platforms as well.  Your change prevents that happening.
Ah, well i have to get used to this "new" devicetree thingy, where one size 
fits all...

Still if you boot a single process system which has FIQ available and has a 
GIC with such a kernel, then you also reprogramm the IPI's as FIQs. But i 
guess thats not a problem as Linux does not self IPI the kernel as other os'es
do?

Best regards
Tim
Daniel Thompson Dec. 1, 2014, 2:13 p.m. UTC | #3
On 01/12/14 13:54, Tim Sander wrote:
>> Look, in my mind it is very simple.  If you are using CONFIG_FIQ on a
>> SMP platform, your life will be very difficult.  The FIQ code enabled
>> by that symbol is not designed to be used on SMP systems, *period*.
> Well the only extra thing you had to do is set up the FIQ registers on every 
> cpu, but i would not call that very difficult. Other than that i am not aware of 
> any problems that are not also present on a uniprocessor system. So i have a 
> hard time following your reasoning why SMP is different from UP in regard to 
> the CONFIG_FIQ.
> 
>> If you decide to enable CONFIG_FIQ, and you use that code on a SMP
>> platform, I'm going to say right now so it's totally clear: if you
>> encounter a problem, I don't want to know about it.  The code is not
>> designed for use on that situation.
> Even with using the FIQ on a Linux SMP system you have not heard from me 
> before, as i knew that this is not your problem (and that is not to say that 
> there where none!). The only interface Linux has been making available is 
> set_fiq_handler. So it was clear that the FIQ is its own domain otherwise 
> untouched by the kernel. Now the line gets blurried with the linux kernel 
> moving to use the FIQ. And with the descicions forthcoming its not only 
> grabbing land it also claims a previous public path for its own. So it doesn't 
> help that its planting some flowers along the way. So please be nice to the 
> natural inhabitants...

Surely only upstream code could claim to be a natural inhabitant.

Whenever I've been working on code that, for whatever reason, cannot be
upstreamed I'd probably best be regarded as a tourist.


> And i really don't get it, that neither ARM nor the kernel community sees fast 
> interrupts as a worthwhile usecase. Unfortunatly the interrupt latencies with 
> Linux are at least a order of magnitude higher than the pure hardware even 
> with longer pipelines can deliver. 
> 
>> Therefore, as far as I'm concerned, the two facilities are mututally
>> exclusive.
> Well can't have the cake and eat it too.
> 
>> I had thought about whether the IPI FIQ should be disabled when a
>> replacement FIQ handler is installed, I deem it not to be a use case
>> that the mainline kernel needs to be concerned about.
> That would be nice.

Just to be clear, this is exactly the dynamic switching that I mentioned
a couple of mails ago.

As I said such code should not especially hard to write but, with the
current mainline kernel, the code would be unreachable and, as a result,
likely also to be more or less untested.


>>> Yes, but if the FIQ handler is also used for IPI, set_fiq_handler gets IPI
>>> interrupts (with the patch starting this thread)? So i think that the
>>> patch
>>> needs to look like:
>>> --- a/arch/arm/kernel/traps.c
>>> +++ b/arch/arm/kernel/traps.c
>>> @@ -483,6 +483,9 @@ asmlinkage void __exception_irq_entry
>>> handle_fiq_as_nmi(struct pt_regs *regs)
>>> +#ifndef CONFIG_FIQ
>>>
>>>  #ifdef CONFIG_ARM_GIC
>>>  
>>>         gic_handle_fiq_ipi();
>>>  
>>>  #endif
>>>
>>> +#endif
>>
>> No.  With a single zImage kernel, you could very well have SMP and FIQ
>> both enabled, but have a non-SMP platform using FIQ, but also support
>> SMP platforms as well.  Your change prevents that happening.
> Ah, well i have to get used to this "new" devicetree thingy, where one size 
> fits all...
> 
> Still if you boot a single process system which has FIQ available and has a 
> GIC with such a kernel, then you also reprogramm the IPI's as FIQs. But i 
> guess thats not a problem as Linux does not self IPI the kernel as other os'es
> do?


> 
> Best regards
> Tim
>
Russell King - ARM Linux Dec. 1, 2014, 3:02 p.m. UTC | #4
On Mon, Dec 01, 2014 at 02:54:10PM +0100, Tim Sander wrote:
> Hi Russell
> 
> Am Montag, 1. Dezember 2014, 10:38:32 schrieb Russell King - ARM Linux:
> > That whole paragraph doesn't make much sense to me.
> > 
> > Look, in my mind it is very simple.  If you are using CONFIG_FIQ on a
> > SMP platform, your life will be very difficult.  The FIQ code enabled
> > by that symbol is not designed to be used on SMP systems, *period*.
>
> Well the only extra thing you had to do is set up the FIQ registers on every 
> cpu, but i would not call that very difficult. Other than that i am not
> aware of any problems that are not also present on a uniprocessor system.
> So i have a hard time following your reasoning why SMP is different from
> UP in regard to the CONFIG_FIQ.

One of the things which FIQ handlers can do is they have their own private
registers which they can modify on each invocation of the FIQ handler -
for example, as a software DMA pointer.

Each CPU has its own private set of FIQ registers, so merely copying the
registers to each CPU will only set their initial state: updates by one
CPU to the register set will not be seen by a different CPU.

> > If you decide to enable CONFIG_FIQ, and you use that code on a SMP
> > platform, I'm going to say right now so it's totally clear: if you
> > encounter a problem, I don't want to know about it.  The code is not
> > designed for use on that situation.
> Even with using the FIQ on a Linux SMP system you have not heard from me 
> before, as i knew that this is not your problem (and that is not to say that 
> there where none!). The only interface Linux has been making available is 
> set_fiq_handler. So it was clear that the FIQ is its own domain otherwise 
> untouched by the kernel.

Correct, because FIQs have very little use in Linux.  They have been used
in the past to implement:
- software DMA to floppy disk controllers (see arch/arm/lib/floppydma.S)
- audio DMA (arch/arm/mach-imx/ssi-fiq.S)
- s2c24xx SPI DMA (drivers/spi/spi-s3c24xx-fiq.S)
- Keyboard (yes, how that qualifies for FIQ I don't know
  (arch/arm/mach-omap1/ams-delta-fiq-handler.S)

The first three do exactly what I describe above, and none of these users
are SMP platforms.  Hence, the FIQ code which we currently have does exactly
what we need it to for the platforms we have.

Now, you're talking about using this in a SMP context - that's a totally
new use for this code which - as I have said several times now - is not
really something that this code is intended to support.

> And i really don't get it, that neither ARM nor the kernel community
> sees fast interrupts as a worthwhile usecase. Unfortunatly the interrupt
> latencies with Linux are at least a order of magnitude higher than the
> pure hardware even with longer pipelines can deliver. 

First point: fast interrupts won't be fast if you load them up with all
the interrupt demux and locking that normal interrupts have; if you
start doing that, then you end up having to make /all/ IRQ-safe locks
in the kernel not only disable normal interrupts, but also disable the
FIQs as well.

At that point, FIQs are no longer "fast" - they will be subject to
exactly the same latencies as normal interrupts.

Second point: we have embraced FIQs where it is appropriate to do so,
but within the restrictions that FIQs present - that is, to keep them
fast, we have to avoid the problem in the first point above, which
means normal C code called from FIQs /can't/ take any kernel lock what
so ever without potentially causing a deadlock.

Even if you think you can (why would UP have locks if it's not SMP)
debugging facilities such as the lock validator will bite you if you
try taking a lock in FIQ context which was already taken in the parent
context.

Third point: FIQs are not available on a lot of ARM platforms.  Hardware
which routes interrupts to FIQs is very limited, normally it's only a
few interrupts which appear there.  Moreover, with more modern platforms
where the kernel runs in the non-secure side, FIQs are /totally/ and
/completely/ unavailable there - FIQs are only available for the secure
monitor to use.

> > No.  With a single zImage kernel, you could very well have SMP and FIQ
> > both enabled, but have a non-SMP platform using FIQ, but also support
> > SMP platforms as well.  Your change prevents that happening.
> 
> Ah, well i have to get used to this "new" devicetree thingy, where one size 
> fits all...

No, you're conflating different things there.  It doesn't have much
to do with DT vs non-DT, because this same problem existed before DT
came along, since there were platforms which could be both UP and SMP.

> Still if you boot a single process system which has FIQ available and has a 
> GIC with such a kernel, then you also reprogramm the IPI's as FIQs. But i 
> guess thats not a problem as Linux does not self IPI the kernel as other os'es
> do?

I'm really sorry, but your above paragraph doesn't make much sense to me.
"single process system" - if there's only one process, there's no point
having a scheduler (it has nothing to schedule) and so I guess you're
not talking about Linux there.

Or do you mean "single processor system" (in other words, uniprocessor or
UP).  In that case, the kernel doesn't use IPIs, because, by definition,
there's no other processors for it to signal to.
Tim Sander Dec. 3, 2014, 1:41 p.m. UTC | #5
Hi Daniel

Am Montag, 1. Dezember 2014, 14:13:52 schrieb Daniel Thompson:
> On 01/12/14 13:54, Tim Sander wrote:
> >> Look, in my mind it is very simple.  If you are using CONFIG_FIQ on a
> >> SMP platform, your life will be very difficult.  The FIQ code enabled
> >> by that symbol is not designed to be used on SMP systems, *period*.
> > 
> > Well the only extra thing you had to do is set up the FIQ registers on
> > every cpu, but i would not call that very difficult. Other than that i am
> > not aware of any problems that are not also present on a uniprocessor
> > system. So i have a hard time following your reasoning why SMP is
> > different from UP in regard to the CONFIG_FIQ.
> > 
> >> If you decide to enable CONFIG_FIQ, and you use that code on a SMP
> >> platform, I'm going to say right now so it's totally clear: if you
> >> encounter a problem, I don't want to know about it.  The code is not
> >> designed for use on that situation.
> > 
> > Even with using the FIQ on a Linux SMP system you have not heard from me
> > before, as i knew that this is not your problem (and that is not to say
> > that there where none!). The only interface Linux has been making
> > available is set_fiq_handler. So it was clear that the FIQ is its own
> > domain otherwise untouched by the kernel. Now the line gets blurried with
> > the linux kernel moving to use the FIQ. And with the descicions
> > forthcoming its not only grabbing land it also claims a previous public
> > path for its own. So it doesn't help that its planting some flowers along
> > the way. So please be nice to the natural inhabitants...
> 
> Surely only upstream code could claim to be a natural inhabitant.
Well from a kernel developer perspective this might be true, but well there 
are things, e.g. the stuff the nice guys at free electrons did, which are quite
reasonable but would be laughed at if tried to include in the kernel:
http://free-electrons.com/blog/fiq-handlers-in-the-arm-linux-kernel/
Still this shows very much that you can build quite powerfull systems which 
combine both the power of linux with the lowes latency the bare hardware can 
give you.

> Whenever I've been working on code that, for whatever reason, cannot be
> upstreamed I'd probably best be regarded as a tourist.
I think that application specific code which needs all the power the hardware 
gives you in a given power envelope and is so optimized for a special usecase 
that integration in kernel makes no sense. So i would hope for a more 
constructive mindset.

> > And i really don't get it, that neither ARM nor the kernel community sees
> > fast interrupts as a worthwhile usecase. Unfortunatly the interrupt
> > latencies with Linux are at least a order of magnitude higher than the
> > pure hardware even with longer pipelines can deliver.
> > 
> >> Therefore, as far as I'm concerned, the two facilities are mututally
> >> exclusive.
> > 
> > Well can't have the cake and eat it too.
> > 
> >> I had thought about whether the IPI FIQ should be disabled when a
> >> replacement FIQ handler is installed, I deem it not to be a use case
> >> that the mainline kernel needs to be concerned about.
> > 
> > That would be nice.
> 
> Just to be clear, this is exactly the dynamic switching that I mentioned
> a couple of mails ago.
Ok, my takeaway is there is currently not enough interest from your side to 
implement it but you would support some changes if submitted?

> As I said such code should not especially hard to write but, with the
> current mainline kernel, the code would be unreachable and, as a result,
> likely also to be more or less untested.
Well, my misconception was, that this might be done by adding some ifdefs
but as Russell pointed out, that is not the way to go.

Best regards
 Tim
Daniel Thompson Dec. 3, 2014, 2:53 p.m. UTC | #6
On 03/12/14 13:41, Tim Sander wrote:
>>> Even with using the FIQ on a Linux SMP system you have not heard from me
>>> before, as i knew that this is not your problem (and that is not to say
>>> that there where none!). The only interface Linux has been making
>>> available is set_fiq_handler. So it was clear that the FIQ is its own
>>> domain otherwise untouched by the kernel. Now the line gets blurried with
>>> the linux kernel moving to use the FIQ. And with the descicions
>>> forthcoming its not only grabbing land it also claims a previous public
>>> path for its own. So it doesn't help that its planting some flowers along
>>> the way. So please be nice to the natural inhabitants...
>>
>> Surely only upstream code could claim to be a natural inhabitant.
> Well from a kernel developer perspective this might be true, but well there 
> are things, e.g. the stuff the nice guys at free electrons did, which are quite
> reasonable but would be laughed at if tried to include in the kernel:
> http://free-electrons.com/blog/fiq-handlers-in-the-arm-linux-kernel/
> Still this shows very much that you can build quite powerfull systems which 
> combine both the power of linux with the lowes latency the bare hardware can 
> give you.
> 
>> Whenever I've been working on code that, for whatever reason, cannot be
>> upstreamed I'd probably best be regarded as a tourist.
> I think that application specific code which needs all the power the hardware 
> gives you in a given power envelope and is so optimized for a special usecase 
> that integration in kernel makes no sense. So i would hope for a more 
> constructive mindset.

A bad choice of words on my part (although in truth it remains an
accurate description of my own experience of working on code not
destined to be upstreamed).

However I certainly want to be constructive.

>>> And i really don't get it, that neither ARM nor the kernel community sees
>>> fast interrupts as a worthwhile usecase. Unfortunatly the interrupt
>>> latencies with Linux are at least a order of magnitude higher than the
>>> pure hardware even with longer pipelines can deliver.
>>>
>>>> Therefore, as far as I'm concerned, the two facilities are mututally
>>>> exclusive.
>>>
>>> Well can't have the cake and eat it too.
>>>
>>>> I had thought about whether the IPI FIQ should be disabled when a
>>>> replacement FIQ handler is installed, I deem it not to be a use case
>>>> that the mainline kernel needs to be concerned about.
>>>
>>> That would be nice.
>>
>> Just to be clear, this is exactly the dynamic switching that I mentioned
>> a couple of mails ago.
> Ok, my takeaway is there is currently not enough interest from your side to 
> implement it but you would support some changes if submitted?

I'd take a good look at them (assuming I'm on Cc: or my mail filters
pick them out). I may still have some concerns about testing it in the
absence of an upstream user but otherwise I would expect to be supportive.

>> As I said such code should not especially hard to write but, with the
>> current mainline kernel, the code would be unreachable and, as a result,
>> likely also to be more or less untested.
> Well, my misconception was, that this might be done by adding some ifdefs
> but as Russell pointed out, that is not the way to go.

Whether its dynamic or not, a change that does not provide some benefit
to the upstream kernel is always going to be much harder to sell to the
people who have to maintain it because they derive much benefit from
maintaining it.


Daniel.
Tim Sander Dec. 5, 2014, 4 p.m. UTC | #7
Hi Russell, Thomas

I have some replys below, but i just post my most important question up here, 
which is my current takeaway from this discussion:
Would patches be accepted which -as Daniel Thompson pointed out- dynamically 
switch the FIQ IPI's off when set_fiq_handler is called (given that the FIQ IPI 
patches are to be merged proper beforhand).

Am Montag, 1. Dezember 2014, 15:02:40 schrieb Russell King - ARM Linux:
> On Mon, Dec 01, 2014 at 02:54:10PM +0100, Tim Sander wrote:
> > Hi Russell
> > 
> > Am Montag, 1. Dezember 2014, 10:38:32 schrieb Russell King - ARM Linux:
> > > That whole paragraph doesn't make much sense to me.
> > > 
> > > Look, in my mind it is very simple.  If you are using CONFIG_FIQ on a
> > > SMP platform, your life will be very difficult.  The FIQ code enabled
> > > by that symbol is not designed to be used on SMP systems, *period*.
> > 
> > Well the only extra thing you had to do is set up the FIQ registers on
> > every cpu, but i would not call that very difficult. Other than that i am
> > not aware of any problems that are not also present on a uniprocessor
> > system. So i have a hard time following your reasoning why SMP is
> > different from UP in regard to the CONFIG_FIQ.
> 
> One of the things which FIQ handlers can do is they have their own private
> registers which they can modify on each invocation of the FIQ handler -
> for example, as a software DMA pointer.
> 
> Each CPU has its own private set of FIQ registers, so merely copying the
> registers to each CPU will only set their initial state: updates by one
> CPU to the register set will not be seen by a different CPU.
> 
> > > If you decide to enable CONFIG_FIQ, and you use that code on a SMP
> > > platform, I'm going to say right now so it's totally clear: if you
> > > encounter a problem, I don't want to know about it.  The code is not
> > > designed for use on that situation.
> > 
> > Even with using the FIQ on a Linux SMP system you have not heard from me
> > before, as i knew that this is not your problem (and that is not to say
> > that there where none!). The only interface Linux has been making
> > available is set_fiq_handler. So it was clear that the FIQ is its own
> > domain otherwise untouched by the kernel.
> 
> Correct, because FIQs have very little use in Linux.  They have been used
> in the past to implement:
> - software DMA to floppy disk controllers (see arch/arm/lib/floppydma.S)
> - audio DMA (arch/arm/mach-imx/ssi-fiq.S)
> - s2c24xx SPI DMA (drivers/spi/spi-s3c24xx-fiq.S)
> - Keyboard (yes, how that qualifies for FIQ I don't know
>   (arch/arm/mach-omap1/ams-delta-fiq-handler.S)
> 
> The first three do exactly what I describe above, and none of these users
> are SMP platforms.  Hence, the FIQ code which we currently have does exactly
> what we need it to for the platforms we have.
> 
> Now, you're talking about using this in a SMP context - that's a totally
> new use for this code which - as I have said several times now - is not
> really something that this code is intended to support.
Yes but as i said the only additional problem is the seperate registers for 
each core. Given the quirks the current GIC version 1 this is really  a minor 
problem:
https://lkml.org/lkml/2014/7/15/550


> > And i really don't get it, that neither ARM nor the kernel community
> > sees fast interrupts as a worthwhile usecase. Unfortunatly the interrupt
> > latencies with Linux are at least a order of magnitude higher than the
> > pure hardware even with longer pipelines can deliver.
> 
> First point: fast interrupts won't be fast if you load them up with all
> the interrupt demux and locking that normal interrupts have; if you
> start doing that, then you end up having to make /all/ IRQ-safe locks
> in the kernel not only disable normal interrupts, but also disable the
> FIQs as well.
I just want to have an CPU context where IRQ's are not switched off by Linux.
It would be nice to use Linux infrastructure like printk but thats just not 
that important. And no, i don't wan't to use some IRQ demuxing, Thats why 
i would be nice to disable the FIQ IPI's dynamically if other uses are set.

> At that point, FIQs are no longer "fast" - they will be subject to
> exactly the same latencies as normal interrupts.
Well the main difference i am after, is to have one interrupt which is not 
masked in any way and which is as fast as the hardware can get (which on a 
cortex a9 is depending on implementation between 500ns to a couple of µs).

> Second point: we have embraced FIQs where it is appropriate to do so,
> but within the restrictions that FIQs present - that is, to keep them
> fast, we have to avoid the problem in the first point above, which
> means normal C code called from FIQs /can't/ take any kernel lock what
> so ever without potentially causing a deadlock.
Yes i am aware of that. I think thats one of the main reasons why the FIQ has 
been mainly unused by Linux. 

> Even if you think you can (why would UP have locks if it's not SMP)
> debugging facilities such as the lock validator will bite you if you
> try taking a lock in FIQ context which was already taken in the parent
> context.
Well no Linux context in FIQ at all. Thats why i was using a daisy chained 
normal interrupt to hand of the normal stuff in linux context.
 
> Third point: FIQs are not available on a lot of ARM platforms.  Hardware
> which routes interrupts to FIQs is very limited, normally it's only a
> few interrupts which appear there.  Moreover, with more modern platforms
> where the kernel runs in the non-secure side, FIQs are /totally/ and
> /completely/ unavailable there - FIQs are only available for the secure
> monitor to use.
I am fully aware that ARM started to mix up FIQ and SecureMode, confusing even 
some silicon vendors, which sadly have the FIQ missing. But aside from that i 
know that i.mx6, xilinx zynq, altera soc all have a FIQ available. The only 
one i know missing the FIQ is the Sitara which had the FIQ documented in first 
revisions of the spec (but has not anymore). So from my totally empirical 
unscientific view 3 of 4 cpus have FIQ functionality. 

Or do you mean that the platform is capable of delivering FIQ but have no 
CONFIG_FIQ set. In that case there is indeed only a small fraction which has 
this config option in use.

> > > No.  With a single zImage kernel, you could very well have SMP and FIQ
> > > both enabled, but have a non-SMP platform using FIQ, but also support
> > > SMP platforms as well.  Your change prevents that happening.
> > 
> > Ah, well i have to get used to this "new" devicetree thingy, where one
> > size
> > fits all...
> 
> No, you're conflating different things there.  It doesn't have much
> to do with DT vs non-DT, because this same problem existed before DT
> came along, since there were platforms which could be both UP and SMP.
D'accord.

> > Still if you boot a single process system which has FIQ available and has
> > a
> > GIC with such a kernel, then you also reprogramm the IPI's as FIQs. But i
> > guess thats not a problem as Linux does not self IPI the kernel as other
> > os'es do?
> 
> I'm really sorry, but your above paragraph doesn't make much sense to me.
> "single process system" - if there's only one process, there's no point
> having a scheduler (it has nothing to schedule) and so I guess you're
> not talking about Linux there.
> 
> Or do you mean "single processor system" (in other words, uniprocessor or
> UP).  In that case, the kernel doesn't use IPIs, because, by definition,
> there's no other processors for it to signal to.
I am sorry, mark that to beeing a non native english speaker, i indeed meant a 
single processor system as with a single process i would definetly not bother 
to run linux at all.

Best regards
Tim
diff mbox

Patch

--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -483,6 +483,9 @@  asmlinkage void __exception_irq_entry 
handle_fiq_as_nmi(struct pt_regs *regs)
+#ifndef CONFIG_FIQ
 #ifdef CONFIG_ARM_GIC
        gic_handle_fiq_ipi();
 #endif
+#endif

As otherwise if the platform has CONFIG_SMP and CONFIG_FIQ and CONFIG_ARM_GIC 
the GIC will get reprogrammed to deliver FIQ's to the handler set by