From owner-freebsd-arm@FreeBSD.ORG Tue Jan 20 11:19:42 2015 Return-Path: Delivered-To: freebsd-arm@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id C8BCCB24; Tue, 20 Jan 2015 11:19:42 +0000 (UTC) Received: from mail-qa0-x22e.google.com (mail-qa0-x22e.google.com [IPv6:2607:f8b0:400d:c00::22e]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 7F287E02; Tue, 20 Jan 2015 11:19:42 +0000 (UTC) Received: by mail-qa0-f46.google.com with SMTP id j7so27467012qaq.5; Tue, 20 Jan 2015 03:19:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=26TUq2RQjugVccjwJQ6nccPPxO9kqDb/rQbRFPQ/MBA=; b=gqMoVsRjp697317MgesDstC0B8Mbvc02EJ9Vi6etZEWI/JvTcgrtXCeh//ygkxmHAk w3bfAE84KL2fRbLQn2mMljcIQc/Kdxr/HcznN9SDBFKupUccOmurpxdWNv0y1R0w2sAS 0wORBAl8CqGcnouNMJ94ban8HQeo5FOIHw5SiEk2i8v21GanAC7N4Y6utBwg7Unix7sY tCZd1ioyhbQM2yryRuw+FXSEnf2CHbt0g+jtcNPLE2+1ijNYxmL/yYQdAz0O6RE30UKP ltu1z1DOr9Wlr9ug8AAG9RBzyed105M3gaGWsD1G+cCA6qduxQ6y5NfKoonOKp0s8B6B VIeQ== MIME-Version: 1.0 X-Received: by 10.224.61.1 with SMTP id r1mr36579921qah.0.1421752781620; Tue, 20 Jan 2015 03:19:41 -0800 (PST) Received: by 10.140.82.180 with HTTP; Tue, 20 Jan 2015 03:19:41 -0800 (PST) In-Reply-To: <54BD9794.4080204@freebsd.org> References: <54BA9888.1020303@freebsd.org> <54BD3F86.3010901@freebsd.org> <54BD9794.4080204@freebsd.org> Date: Tue, 20 Jan 2015 12:19:41 +0100 Message-ID: Subject: Re: interrupt framework From: Svatopluk Kraus To: Nathan Whitehorn Content-Type: text/plain; charset=UTF-8 Cc: freebsd-arm@freebsd.org X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: "Porting FreeBSD to ARM processors." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Jan 2015 11:19:42 -0000 On Tue, Jan 20, 2015 at 12:47 AM, Nathan Whitehorn wrote: > > On 01/19/15 15:00, Svatopluk Kraus wrote: >> >> On Mon, Jan 19, 2015 at 6:31 PM, Nathan Whitehorn >> wrote: >>> >>> On 01/19/15 08:42, Svatopluk Kraus wrote: >>>> >>>> On Sat, Jan 17, 2015 at 6:14 PM, Nathan Whitehorn >>>> 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