Date: Thu, 22 Jan 2015 09:32:24 -0800 From: Nathan Whitehorn <nwhitehorn@freebsd.org> To: Svatopluk Kraus <onwahe@gmail.com> Cc: freebsd-arm@freebsd.org Subject: Re: interrupt framework Message-ID: <54C13428.8070003@freebsd.org> In-Reply-To: <CAFHCsPUM5GLARM6YOtDcF=4HpCEszwAAkFtTupcfGOROOiDuXg@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> <54BE7E6D.6060800@freebsd.org> <CAFHCsPUM5GLARM6YOtDcF=4HpCEszwAAkFtTupcfGOROOiDuXg@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 01/22/15 08:59, Svatopluk Kraus wrote: > On Tue, Jan 20, 2015 at 5:12 PM, Nathan Whitehorn > <nwhitehorn@freebsd.org> wrote: >> 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 > > As PIC should be always identified somehow and it makes framework more > simple, I've finally decided to make the change, so PIC is now > identified by some opaque key which I named xref and gave it intptr_t > type. However, in the light of Andrew's message about existence of not > 100% FDT system, the uniqueness of the key could be an issue. Adding > something like key type may be a solution. > > Today, I made intrng more complete. I implemented fully the interrupt > naming and counting (IPI included). However, it will not work well in > case of removable PICs now. I'm attaching new version. > > Svata Thanks! This looks good from my end. -Nathan
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?54C13428.8070003>