Date: Tue, 20 Jan 2015 08:12:29 -0800 From: Nathan Whitehorn <nwhitehorn@freebsd.org> To: Svatopluk Kraus <onwahe@gmail.com> Cc: freebsd-arm@freebsd.org Subject: Re: interrupt framework Message-ID: <54BE7E6D.6060800@freebsd.org> In-Reply-To: <CAFHCsPXLvwJ_5FJaeoKSHjbgmtwzSXFtuPr1h=bO1g9tghyDog@mail.gmail.com> References: <CAFHCsPX5kG_v-F-cjpyMQsT_b386eok=mqWW0%2BEUb_4-_1Otnw@mail.gmail.com> <54BA9888.1020303@freebsd.org> <CAFHCsPX-X-OG4jGLbhdH1BVtqorJKUeaVbzabX-%2BUfEM2fhD6A@mail.gmail.com> <54BD3F86.3010901@freebsd.org> <CAFHCsPUqq-o4z9c5_8SYxcefUiFvGADB5FnB5NiQuu6XBrdyng@mail.gmail.com> <54BD9794.4080204@freebsd.org> <CAFHCsPXLvwJ_5FJaeoKSHjbgmtwzSXFtuPr1h=bO1g9tghyDog@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 01/20/15 03:19, Svatopluk Kraus wrote: > On Tue, Jan 20, 2015 at 12:47 AM, Nathan Whitehorn > <nwhitehorn@freebsd.org> wrote: >> On 01/19/15 15:00, Svatopluk Kraus wrote: >>> On Mon, Jan 19, 2015 at 6:31 PM, Nathan Whitehorn >>> <nwhitehorn@freebsd.org> wrote: >>>> On 01/19/15 08:42, Svatopluk Kraus wrote: >>>>> On Sat, Jan 17, 2015 at 6:14 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. >>>>>>> >>>>>>> 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. >>>>>>> >>>>>>> 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(). >>>>>>> >>>>>>> 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. >>>>>>> >>>>>>> Svata >>>>>>> >>>>>> I took a look through intrng.c and had a couple comments about the FDT >>>>>> mapping stuff: >>>>>> >>>>>> 1. You use the device tree node handles as lookup keys rather than xref >>>>>> handles. These are not necessarily stable, so you should use xref >>>>>> handles >>>>>> instead. >>>>>> >>>>>> 2. If you make change (1), you don't depend on any OF_* stuff and can >>>>>> use >>>>>> the same code with the PIC node ID as an opaque key on non-FDT >>>>>> platforms. >>>>>> We >>>>>> do this on PowerPC as well, which has been very useful. It will also >>>>>> save >>>>>> some #ifdef. >>>>>> -Nathan >>>>>> >>>>> Thanks. I did changes due to (1). Considering (2), I understand what >>>>> you are doing in PowerPC, but it's not something I could adapt so >>>>> easily. Hiding phandle_t behind uint32_t is clever, saves a few FDT >>>>> #ifdefs, but makes things a little mysterious. Even if we will think >>>>> about this uint32_t like some kind of key, there should be a function >>>>> which convert phandle_t to that uint32_t key. >>>>> >>>>> I'm attaching new version of intrng.c with change (1) and with some >>>>> more little adjustments. >>>>> >>>>> Svata >>>> >>>> Thanks! How do you plan to support multiple PICs on non-FDT platforms >>>> then? >>>> It looks like it just fails at the moment. >>>> -Nathan >>> >>> There is the following mapping function: >>> u_int arm_namespace_map_irq(device_t dev, uint8_t type, uint16_t num); >>> >>> I named it "namespace" but it can be named another way. I think it >>> does same like in PowerPC when node is NULL. However, there is one >>> more argument - type. For example, it's used for IPI mapping in >>> intrng.c. >>> >>> Svata >>> >> So you need the PIC's device_t to allocate an interrupt? That doesn't seem >> workable in the real world. What's wrong with just exposing the FDT >> interface with the phandle_t as an opaque key? You don't do anything with it >> except use it as a table lookup key, so it does not in any way matter what >> it actually is. >> -Nathan > > Yes, some identification of a PIC is needed always. In FDT case, xref > is that identification. In not FDT case, I thought that PIC's device_t > should be that identification. If you are saying that PIC's device_t > cannot be used for in some cases, then some else identification must > be used and associated mapping function too. I already wrote about > that before. But to be clear, I have no problem with opaque key to > identify a PIC. I have a problem with how to ensure that the key is > unique. IMHO, it's not good to mix FDT xref type of a key with various > other types of a key and hope that the identification is still > correct. Some rules have to be definined ar least. > > Tell me, how do you think a PIC should be identified with neither > device_t nor FDT xref? > > FYI, I was hoping that FDT xref is a kernel virtual address which is > unique itself enough. But I was told that FDT xref is more like > offset. > > Svata > Right, it's just a number, though a guaranteed unique one within the device tree. Usually, a system is going to be 100% FDT based or 100% non-FDT based, so you don't have to worry about collisions. This is the case, for example, for ACPI. What we've done on PowerPC is that some platform logic is responsible for maintaining uniqueness of identifiers that knows about whatever hardware mapping scheme there is. Andrew can probably comment on whether a uint32_t will be easy to work with for ACPI. -Nathan
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?54BE7E6D.6060800>