Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 15 Jan 2015 23:42:53 +0100
From:      Svatopluk Kraus <onwahe@gmail.com>
To:        Nathan Whitehorn <nwhitehorn@freebsd.org>
Cc:        freebsd-arm@freebsd.org
Subject:   Re: interrupt framework
Message-ID:  <CAFHCsPXfKw6Cf1Mjqr=acwSYrs-5gu1Bs1bXK0n6prTo65LNVA@mail.gmail.com>
In-Reply-To: <54B7EB67.6010702@freebsd.org>
References:  <CAFHCsPX5kG_v-F-cjpyMQsT_b386eok=mqWW0%2BEUb_4-_1Otnw@mail.gmail.com> <54B7EB67.6010702@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Jan 15, 2015 at 5:31 PM, Nathan Whitehorn
<nwhitehorn@freebsd.org> wrote:
>
> On 01/15/15 05:51, Svatopluk Kraus wrote:
>>
>> Hi community,
>>
>> I and Michal Meloun have done some work on ARM interrupt framework and
>> this is the result.
>>
>> We've started with intrng project with Ian's WIP changes, have looked
>> at Andrew's ARM64 git repository, and this is how we think an
>> interrupt framework should look like. We've implemented it with
>> removable interrupt controllers in mind (PCI world). It's not finished
>> from this point of view, however some functions are more complex
>> because of it.
>>
>> It's tested on pandaboard and only GIC is implemented now. There is no
>> problem to implement it to other controllers. We are open to questions
>> and can finish our work considering any comments. Whoever is waiting
>> for ARM interrupt framework as we were, you are welcome to test it.
>> Whoever is welcome. The patches are done against FreeBSD-11-current
>> revision 277210. There are two new files.
>>
>> ARM_INTRNG option must be added to board configuration file for new
>> framework.
>>
>> There are still some things not implemented and some things which
>> should be discussed like PPI support. For example, how to enable PPI
>> interrupt on other CPUs when they are already running?
>>
>> We keep in mind that an interrupt framework should be helpfull but
>> general enough to not dictate interrupt controlles too much. Thus we
>> try to keep some things as much separated as possible. Each interrupt
>> is represented by an interrupt source (ISRC) in the framework. An ISRC
>> is described by an interrupt number which is much more an unique
>> resource handle - totally independent on internal representation of
>> interrupts in any interrupt controller.
>
>
> This is a good design.
>
>> An interrupt is described by cells in FDT world. The cells can be
>> decoded only by associated interrupt controller and as such, they are
>> transparent for interrupt framework. The framework provides
>> arm_fdt_map_irq() function which maps this transparent cells to an
>> interrupt number. It creates an ISRC, saves cells on it, and once when
>> associated interrupt controller is registered, it provides the ISRC
>> with cells into the controller.
>>
>> It's a controller responsibility to save an ISRC associated with
>> cells. An ISRC is transparent for any controller. However, an
>> controller can set/get its data to/from an ISRC. Further, an
>> controller should set a name to an ISRC according to internal
>> representation of associated interrupt.
>
>
> I'm assuming you are referring to arm_describe_irq? I had three comments on
> that:
> 1. Can you please make it part of the same API as ofw_bus_map_irq() rather
> than an out-of-band thing? This will let other platforms use it.
> 2. Once you've done that, you can remove all the #ifdef from simplebus. I
> would strongly prefer it not gain platform-specific #ifdef.
> 3. We should not be adding new things to machine/fdt.h unless they are
> really internal platforms APIs for reading the FDT. And even then, the
> header should probably disappear soon. FDT_MAP_IRQ() is used only in
> arm/intr.c, for example, and should be replaced by a direct invocation of
> arm_fdt_map_intr(). arm_describe_irq() should not join it there.
>


Hmm, we was not aware of that as we have taken something what was
already designed this way. However, it's not problem to arrange things
according to you comments.


>> An controller interrupt dispatch function can call framework only if
>> it has associated ISRC to received interrupt.
>>
>> For legacy reason, there is arm_namespace_map_irq() function. An
>> interrupt is described by namespace type and a number from the
>> namespace. It's intented for use with no FDT drivers. Now, it's used
>> for mapping an IPI on a controller.
>>
>> We think that it's better to call chained controllers (with filter
>> only) without MI interrupt framework overhead, so we implemented
>> shortcut. It could be utilized by INTR_SOLO flag during
>> bus_setup_intr().
>
>
> Can you describe the shortcut? We never ended up needing something like this
> on PowerPC.
>


In standard MI interrupt framework, an interrupt event is created for
an interrupt and intr_event_handle() is called to handle an interrupt
when it comes. Many things happen in this functions before real
filters and/or handlers are called. In former design, when controllers
are chained, it's done on each of them - on each layer. So if a
controller is fast enough and handles interrupts by a filter, we can
call the filter without MI framework overhead. Thus we save time and
stack space. And we do not have a problem to pass trapframe thru
chained controllers. Further, it can be utilized by any interrupt
where time is critical.


>> Only an interrupt controller can really know its position in interrupt
>> controller's tree. So root controller must claim itself as a root. In
>> FDT world, according to ePAPR approved version 1.1 from 08 April 2011,
>> page 30:
>>
>> "The root of the interrupt tree is determined when traversal of the
>> interrupt tree reaches an interrupt controller node without an
>> interrupts property and thus no explicit interrupt parent."
>>
>> Thus there are no need for any non-standard things in DTS files.
>
>
> Right, this is what we did on PPC.
> -Nathan
>
>> Svata
>>
>>
>> _______________________________________________
>> freebsd-arm@freebsd.org mailing list
>> http://lists.freebsd.org/mailman/listinfo/freebsd-arm
>> To unsubscribe, send any mail to "freebsd-arm-unsubscribe@freebsd.org"
>
>
> _______________________________________________
> freebsd-arm@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-arm
> To unsubscribe, send any mail to "freebsd-arm-unsubscribe@freebsd.org"



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAFHCsPXfKw6Cf1Mjqr=acwSYrs-5gu1Bs1bXK0n6prTo65LNVA>