From owner-freebsd-arch@freebsd.org Sun Jul 24 22:29:42 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 6D863BA30AB; Sun, 24 Jul 2016 22:29:42 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from d.mail.sonic.net (d.mail.sonic.net [64.142.111.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 5B8411C9D; Sun, 24 Jul 2016 22:29:42 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from zeppelin.tachypleus.net (75-101-50-44.static.sonic.net [75.101.50.44]) (authenticated bits=0) by d.mail.sonic.net (8.15.1/8.15.1) with ESMTPSA id u6OMTdKZ021282 (version=TLSv1.2 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Sun, 24 Jul 2016 15:29:39 -0700 To: freebsd-arch , freebsd-arm@freebsd.org, Michal Meloun , Svatopluk Kraus From: Nathan Whitehorn Subject: bus_map_intr() changes Message-ID: Date: Sun, 24 Jul 2016 15:29:38 -0700 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:45.0) Gecko/20100101 Thunderbird/45.1.0 MIME-Version: 1.0 X-Sonic-CAuth: UmFuZG9tSVYpR/a0cWS68gL1SqUzzrY3bq00D+HhJ3yL9Mq/FCQoWRnSA7uRKVpReCrqnhptsLMbquuh7pe5qLO8KbrZ96y06DKL7GPCMhk= X-Sonic-ID: C;JHm6G+5R5hGIN5NwxPCmMQ== M;uDX2G+5R5hGIN5NwxPCmMQ== X-Spam-Flag: No X-Sonic-Spam-Details: 0.0/5.0 by cerberusd Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.22 X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 24 Jul 2016 22:29:42 -0000 There is a very long thread on the SVN list about this that has lasted much too long and should be over here. So, I'd like to start with a clean slate. The discussion is related to r301453, which adds a function BUS_MAP_INTR() and a function bus_extend_resource() that allow a parent bus to decode SYS_RES_IRQ-type resources during bus_alloc_resource(). This is designed to allow the parent bus to read in flags from the device tree regarding IRQ setup (polarity, trigger mode) and pass that along to the interrupt controller with the resource allocation request. By specifying an enum intr_map_data_type, you can also specify what kind of interrupt specifier is being added to the decoration (ACPI, FDT, GPIO). This is a departure from the existing code used on device tree systems (OFW_BUS_MAP_INTR()), which allocates a virtual IRQ corresponding to an interrupt-parent key and one of the device tree's opaque arbitrary-length interrupt specifiers at resource assignment time. The information that virtual IRQ maps to is cached and then applied by the PIC when the PIC's interrupts are configured, which may be after BUS_SETUP_INTR() if the call is made before interrupts are on and the PIC hasn't attached at that point. I am making a wiki page at https://wiki.freebsd.org/Complicated_Interrupts to describe the current implementation and the rationale for its implementation, which should exist by the time most read this. Some of that content is copied below for ease of replying. My concern is that the new API (r301453) parallels the existing one in a way that will require both to be maintained indefinitely, while providing less functionality, in particular in three ways: 1. Breaking the opacity of the device tree's interrupt specifiers (which only the PIC driver knows what to do with) 2. Requiring the bus parent to know exactly how to map an IRQ number (poorly determined, as above) to a device tree entry (which may not include it -- see below) and know how to interpret it (above, which only the PIC driver knows) 3. It also requires that the PIC driver already be attached, which cannot be guaranteed on some systems where the PIC is a bus child of devices with interrupt on that same PIC. What I would like to establish here, rather than just being cranky, is that this new API both (a) does something on real hardware that the existing API cannot do, either currently or with trivial modifications and (b) is capable of expressing the things the current API can express. If the answer to either of those is "no", we're going to have to support both in perpetuity, with different paths on different platforms and the whole thing is going to be a huge mess. We're in a situation right now where that will be baked into FreeBSD 11 for the duration of the branch, which is quite unfortunate. -Nathan ---- Excerpt of wiki text ---- ---- Part 1: Overview of mechanism ---- The core part of this system is a registry in machine-dependent code that maps some description of an interrupt to an IRQ number used by the rest of the kernel. This number is arbitrary; on systems in which a useful human-readable number can be extracted in a general way from the description, it is helpful for users for the IRQ number to be related to something about the system (e.g. the interrupt pin on single-controller systems) but it can be just a monotonically increasing integer. Currently one interrupt mapping strategy is implemented: Open Firmware (or FDT) interrupt-parent / interrupt specifier tuples to IRQ. Bus code maps the Open Firmware interrupt specifier using the ofw_bus_map_intr() function, which is cascaded through the bus hierarchy and is usually resolved by nexus. int ofw_bus_map_intr(device_t dev, phandle_t iparent, int icells, pcell_t *intr); This takes the requesting device, the xref phandle of the interrupt parent (e.g. from the "interrupt-parent" property, or the equivalent entry in an interrupt-map) and the byte string describing the interrupt (e.g. the contents of the "interrupts" property, or the equivalent entry in an interrupt-map) and returns a unique IRQ number that can be added to a resource list and used with bus_alloc_resource(), bus_setup_intr(), etc. In the event you needed more than OF-type mappings (e.g. for ACPI) you could add an equivalent acpi_bus_map_intr() method to nexus that tabulates mappings in parallel based on different data. ---- Part 2: Rationale ---- As a separate issue, it would be great if you could comment on a way to implement the following two scenarios with this API, which I think are currently impossible and would need to solved to avoid bifurcation. These kinds of things are what drove the current API. --- Case 1: the G5 Powermac --- I have hardware with two PICs, one cascaded from the other. PIC 1 lives in the northbridge, and PIC 2 lives on a device on the PCI bus behind a couple of bridges. Depending on the era of the hardware, these are cascaded in different directions. They have different interrupt specifier formats. A. How do I represent interrupts on the PCI bus parent of PIC 2 that are handled by PIC 2? PIC 2 obviously can't attach before its bus parent, but the bus parent can't complete initialization without the ability to setup its interrupts. B. Devices on the PCI bus have interrupts handled by a mixture of PIC 1 and PIC 2, sometimes on the same device and not always expressable through the bus hierarchy. For example, one of the two storage controllers has an interrupt on PIC 2 run through a wire that doesn't go through the PCI connection and so isn't in the interrupt-map of the PCI bus, which is wired (mostly) to PIC 1, and about which the parent bus can and should know nothing. --- Case 2: IBM OPAL firmware --- The /ibm,opal device on IBM PowerNV systems has a non-standard interrupts property ("opal-interrupts") that contains the list of IRQs that should be forwarded to the firmware. These are not interrupts belonging to a single physical device at /ibm,opal (which is a virtual device anyway) and so are not in the interrupts property; nor do they necessarily share an interrupt parent. How do I represent this? --- Case 3: IBM XICS interrupts --- On virtualized (and most non-virtualized) IBM hardware, the interrupts are one cell and that single cell encodes the interrupt parent, the line sense, and the IRQ. --- Case 4: MSIs --- MSIs are assigned purely by the PCI bus and the PCI bus parent can't know about them from the device tree. How does the bus parent sensibly decorate resources like this? The PCI MSI API assumes that these all exist purely as 32-bit integers and they are not assigned through resource lists in the conventional fashion. From owner-freebsd-arch@freebsd.org Mon Jul 25 13:34:05 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 7CBBCBA416A for ; Mon, 25 Jul 2016 13:34:05 +0000 (UTC) (envelope-from mmel@FreeBSD.org) Received: from mail.miracle.cz (mail.miracle.cz [193.84.128.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mail.miracle.cz", Issuer "Miracle Group Root CA" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 1E69A179D; Mon, 25 Jul 2016 13:34:04 +0000 (UTC) (envelope-from mmel@FreeBSD.org) Received: from [193.84.128.50] (meloun.ad.miracle.cz [193.84.128.50]) by mail.miracle.cz (Postfix) with ESMTPSA id 90BFD3ACF0; Mon, 25 Jul 2016 15:34:01 +0200 (CEST) Subject: Re: INTRNG (Was: svn commit: r301453....) To: Nathan Whitehorn , Svatopluk Kraus , freebsd-arch@freebsd.org References: <201606051620.u55GKD5S066398@repo.freebsd.org> <578E0B5D.3070105@FreeBSD.org> <578F6075.7010500@FreeBSD.org> <05a80ac6-4285-ec9d-36e9-2f92c609f746@freebsd.org> <57907B0F.9070204@FreeBSD.org> <9d2a224c-b787-2875-5984-a7a2354e8695@freebsd.org> <57934ABD.6010807@FreeBSD.org> <4e7a3e8f-cc21-f5f2-e3e0-4dbd554a4cd0@freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> From: Michal Meloun X-Enigmail-Draft-Status: N1110 Message-ID: <57961549.4020105@FreeBSD.org> Date: Mon, 25 Jul 2016 15:34:01 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.4.3 (mail.miracle.cz); Mon, 25 Jul 2016 15:34:01 +0200 (CEST) X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 Jul 2016 13:34:05 -0000 It seems that you accidentally moved this thread to arm@, so I moved him again to arch@. Dne 25.07.2016 v 0:38 Nathan Whitehorn napsal(a): > > > On 07/24/16 10:51, Michal Meloun wrote: > > [snip] > >>>> >>>>> >>>>>> For this reason we makes domain key composite, in our model, the >>>>>> domain key consist of "domain key type" >>>>>> and "domain key value". This composite key guarantees uniqueness >>>>>> and it also allows to select proper parser for byte string. >>>>> >>>>> Yes, but this solves what is a nonexistant problem by making the >>>>> system substantially less flexible and more invasive. Which is not >>>>> a good tradeoff. >>>>> >>>> I think that existence of problem is confirmed in the above example . >>>> Quote from previous paragraph: >>>> "We could solve this in a number of ways, ... , or adding another >>>> value (domain type, for example)." >>>> What can I say more ... >>> >>> Except that the example you gave *is not an example* of the problem >>> you are describing. You would only end up with a problem if: >>> 1) You had interrupts in a GPIO property rather than in an >>> interrupts property (or equivalent). >>> 2) *And* you had interrupts on GPIOs in an interrupts property (or >>> equivalent) >>> 3) *and* those are encoded differently >>> 4) *and* the different encodings use the same number of cells >>> 5) *and* are not otherwise distinguishable >>> >>> Does that ever actually happen, in the real world, on any device >>> tree? You could imagine any kind of messed up thing you want, but we >>> shouldn't structure APIs around them, especially given that the >>> current alternative you are proposing has real, concrete problems on >>> real hardware that actually exists. >>> >> Allow me to respond to this issue only. I think that this main >> issue, everything else looks resolvable for me (or, in worst case, >> can be converted to MD code). >> >> from linux/arch/arm/boot/dts/am335x-evm.dts (it's first file that I'm >> searched) >> >> ---------------------------------------------------------------------------------------------------------------------------- >> >> &gpmc { >> status = "okay"; >> pinctrl-names = "default"; >> pinctrl-0 = <&nandflash_pins_s0>; >> ranges = <0 0 0x08000000 0x1000000>; /* CS0: 16MB for NAND */ >> nand@0,0 { >> compatible = "ti,omap2-nand"; >> reg = <0 0 4>; /* CS0, offset 0, IO size 4 */ >> interrupt-parent = <&gpmc>; >> interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */ >> <1 IRQ_TYPE_NONE>; /* termcount */ >> rb-gpios = <&gpmc 0 GPIO_ACTIVE_HIGH>; /* gpmc_wait0 */ >> ---------------------------------------------------------------------------------------------------------------------------- >> >> so we have >> rb-gpios = <&gpmc 0 GPIO_ACTIVE_HIGH>; /* gpmc_wait0 */ >> and >> interrupt-parent = <&gpmc>; >> interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */ >> >> >> --- or --- >> from linux/arch/arm/boot/dts/exynos5420-peach-pit.dts >> >> max98090: codec@10 { >> compatible = "maxim,max98090"; >> reg = <0x10>; >> interrupts = <2 0>; >> interrupt-parent = <&gpx0>; >> pinctrl-names = "default"; >> and >> sleep-gpios = <&gpx3 5 GPIO_ACTIVE_HIGH>; >> >> also, in this case, first cell in 'interrupts' property have >> different meaning that second cell in 'sleep-gpio'. >> (on exynos, not all gpios supports interrrupts) >> >> >> In general, binding for 'interrupts ' and '-gpios ' are >> different, but can have same size . > > Thanks so much for the concrete example! This is very helpful. Three > questions: > > 1. Are the drivers for these devices actually taking interrupts on the > lines defined in the *-gpios property? It seems weird to me that there > would be interrupts on GPIOs and, simultaneously, implicit interrupts > on other GPIOs. It looks like the things on the *-gpios properties are > things you set rather than things you would request an interrupt on > (sleep for the second case, a wait pin in the first). Yes, interrupts on GPIO pins are relatively widely used. For example, SDHCI driver uses this for handling of card detect pin. Moreover, the SDHCI driver have zero knowledge about given GPIO pin interrupt capabilities. He simply tries to allocate interruption, and when it fails, it switches to a pooling mode. Other usage is HDMI cable/monitor detect input, USB VBUS pin or so... > 2. What is the different meaning? I assumed the second cell in both > cases is the GPIO pin number, because anything else would be crazy. No, on exynos (if memory serves me), not all GPIOs have interrupt capability, and they are numbered independently. But again, please note that both *-gpios and interrupts properties are defined independently, they can be formatted differently even within a single controller. > > 3. What on earth is IRQ_TYPE_NONE? That's an OpenPIC flag that just > disables the interrupt entirely. Why would you want to configure > things that way? In Linux DT, the IRQ_TYPE_NONE is equivalent of our pair -> don't change actual interrupt configuration, i want to reuse default or already preset configuration (from uboot for example). > > (#1 is the only one that really matters here; the rest are personal > curiosity) > > If the answer to (1) is "yes", there would be a good case for adding > something like a ofw_bus_map_gpio_as_intr() or the like that takes the > same arguments as ofw_bus_map_intr() and propagates them to the > control in some way but with a flag. It's an easy extension and this > is the reason the function has "ofw" in the name. My one qualm about > that is that there is no guarantee that a GPIO controller can actually > provide interrupts on a random GPIO (nor is it clear that that is > actually what is wanted here). That qualm applies to both APIs, of > course, and I still don't see a rationale for throwing away the > existing code even if (1) is true. But current INTRNG is no longer OFW centric, it can easily handle any other 'configuration source'. We removed it (from INTRNG core) at r297539, at request from MIPS folks. But again, all what I want in this area is (for me) simple change of your ofw_bus_map_intr() method to something like: enum intr_map_data_type { INTR_MAP_DATA_OFW, INTR_MAP_DATA_GPIO, ... }; int bus_map_intr(device_t dev, enum intr_map_data_type type, uintptr_t pic_id, void *config_data, int config_size) Please, don't take current implementation of bus_map_intr() in account. >From my current point of view, the method is badly named and his name doesn't reflect its functionality. Michal > > I've changed the mailing list here since the SVN list really isn't the > best place. For people who want context and are seeing this for the > first time, please see the other email I just sent. > -Nathan From owner-freebsd-arch@freebsd.org Mon Jul 25 14:05:05 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 96097BA46E1; Mon, 25 Jul 2016 14:05:05 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from c.mail.sonic.net (c.mail.sonic.net [64.142.111.80]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 7F74F16AE; Mon, 25 Jul 2016 14:05:05 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from zeppelin.tachypleus.net (75-101-50-44.static.sonic.net [75.101.50.44]) (authenticated bits=0) by c.mail.sonic.net (8.15.1/8.15.1) with ESMTPSA id u6PE52Um020787 (version=TLSv1.2 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Mon, 25 Jul 2016 07:05:02 -0700 Subject: Re: INTRNG (Was: svn commit: r301453....) To: Michal Meloun , Svatopluk Kraus , freebsd-arch@FreeBSD.org, freebsd-arm@freebsd.org References: <201606051620.u55GKD5S066398@repo.freebsd.org> <578E0B5D.3070105@FreeBSD.org> <578F6075.7010500@FreeBSD.org> <05a80ac6-4285-ec9d-36e9-2f92c609f746@freebsd.org> <57907B0F.9070204@FreeBSD.org> <9d2a224c-b787-2875-5984-a7a2354e8695@freebsd.org> <57934ABD.6010807@FreeBSD.org> <4e7a3e8f-cc21-f5f2-e3e0-4dbd554a4cd0@freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <57961549.4020105@FreeBSD.org> From: Nathan Whitehorn Message-ID: Date: Mon, 25 Jul 2016 07:05:02 -0700 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:45.0) Gecko/20100101 Thunderbird/45.1.0 MIME-Version: 1.0 In-Reply-To: <57961549.4020105@FreeBSD.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Sonic-CAuth: UmFuZG9tSVaqp7NbCWPh2Ab+BkUZZm/pbb6akU/7Rutiv9ygsDsqYQdDj8LiSnsKVToCW/Dc7TNQMww24dN4Wa08iimDR5HHv7edEWKpQko= X-Sonic-ID: C;DnHbx3BS5hGpaJtMTlz00w== M;Ms0oyHBS5hGpaJtMTlz00w== X-Spam-Flag: No X-Sonic-Spam-Details: 0.0/5.0 by cerberusd X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 Jul 2016 14:05:05 -0000 On 07/25/16 06:34, Michal Meloun wrote: > It seems that you accidentally moved this thread to arm@, so I moved him > again to arch@. (Thanks -- I meant it to be both arm@ and arch@, like the other email) > Dne 25.07.2016 v 0:38 Nathan Whitehorn napsal(a): >> >> On 07/24/16 10:51, Michal Meloun wrote: >> >> [snip] >> >>>>>>> For this reason we makes domain key composite, in our model, the >>>>>>> domain key consist of "domain key type" >>>>>>> and "domain key value". This composite key guarantees uniqueness >>>>>>> and it also allows to select proper parser for byte string. >>>>>> Yes, but this solves what is a nonexistant problem by making the >>>>>> system substantially less flexible and more invasive. Which is not >>>>>> a good tradeoff. >>>>>> >>>>> I think that existence of problem is confirmed in the above example . >>>>> Quote from previous paragraph: >>>>> "We could solve this in a number of ways, ... , or adding another >>>>> value (domain type, for example)." >>>>> What can I say more ... >>>> Except that the example you gave *is not an example* of the problem >>>> you are describing. You would only end up with a problem if: >>>> 1) You had interrupts in a GPIO property rather than in an >>>> interrupts property (or equivalent). >>>> 2) *And* you had interrupts on GPIOs in an interrupts property (or >>>> equivalent) >>>> 3) *and* those are encoded differently >>>> 4) *and* the different encodings use the same number of cells >>>> 5) *and* are not otherwise distinguishable >>>> >>>> Does that ever actually happen, in the real world, on any device >>>> tree? You could imagine any kind of messed up thing you want, but we >>>> shouldn't structure APIs around them, especially given that the >>>> current alternative you are proposing has real, concrete problems on >>>> real hardware that actually exists. >>>> >>> Allow me to respond to this issue only. I think that this main >>> issue, everything else looks resolvable for me (or, in worst case, >>> can be converted to MD code). >>> >>> from linux/arch/arm/boot/dts/am335x-evm.dts (it's first file that I'm >>> searched) >>> >>> ---------------------------------------------------------------------------------------------------------------------------- >>> >>> &gpmc { >>> status = "okay"; >>> pinctrl-names = "default"; >>> pinctrl-0 = <&nandflash_pins_s0>; >>> ranges = <0 0 0x08000000 0x1000000>; /* CS0: 16MB for NAND */ >>> nand@0,0 { >>> compatible = "ti,omap2-nand"; >>> reg = <0 0 4>; /* CS0, offset 0, IO size 4 */ >>> interrupt-parent = <&gpmc>; >>> interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */ >>> <1 IRQ_TYPE_NONE>; /* termcount */ >>> rb-gpios = <&gpmc 0 GPIO_ACTIVE_HIGH>; /* gpmc_wait0 */ >>> ---------------------------------------------------------------------------------------------------------------------------- >>> >>> so we have >>> rb-gpios = <&gpmc 0 GPIO_ACTIVE_HIGH>; /* gpmc_wait0 */ >>> and >>> interrupt-parent = <&gpmc>; >>> interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */ >>> >>> >>> --- or --- >>> from linux/arch/arm/boot/dts/exynos5420-peach-pit.dts >>> >>> max98090: codec@10 { >>> compatible = "maxim,max98090"; >>> reg = <0x10>; >>> interrupts = <2 0>; >>> interrupt-parent = <&gpx0>; >>> pinctrl-names = "default"; >>> and >>> sleep-gpios = <&gpx3 5 GPIO_ACTIVE_HIGH>; >>> >>> also, in this case, first cell in 'interrupts' property have >>> different meaning that second cell in 'sleep-gpio'. >>> (on exynos, not all gpios supports interrrupts) >>> >>> >>> In general, binding for 'interrupts ' and '-gpios ' are >>> different, but can have same size . >> Thanks so much for the concrete example! This is very helpful. Three >> questions: >> >> 1. Are the drivers for these devices actually taking interrupts on the >> lines defined in the *-gpios property? It seems weird to me that there >> would be interrupts on GPIOs and, simultaneously, implicit interrupts >> on other GPIOs. It looks like the things on the *-gpios properties are >> things you set rather than things you would request an interrupt on >> (sleep for the second case, a wait pin in the first). > Yes, interrupts on GPIO pins are relatively widely used. For example, > SDHCI driver uses this for handling of card detect pin. Moreover, the > SDHCI driver have zero knowledge about given GPIO pin > interrupt capabilities. He simply tries to allocate interruption, and > when it fails, it switches to a pooling mode. > Other usage is HDMI cable/monitor detect input, USB VBUS pin or so... That wasn't my question. Are these particular device drivers allocating interrupts both on the GPIOs in their "interrupts" property (which are entirely GPIOs in this example) *and* on the GPIOs listed as resources but not listed as interrupts? If they are, then you need a switching mechanism, but that seems pretty unlikely given the names of the non-interrupt GPIOs (they look like outputs). It would also be a somewhat deranged way to set up a device tree -- not that that rules it out or anything. >> 2. What is the different meaning? I assumed the second cell in both >> cases is the GPIO pin number, because anything else would be crazy. > No, on exynos (if memory serves me), not all GPIOs have interrupt > capability, and they are numbered independently. > But again, please note that both *-gpios and interrupts properties are > defined independently, they can be formatted differently even within a > single controller. Which is totally fine. The question is if you need to set up interrupts on the GPIOs both the "interrupts" and non-interrupts properties, which is easy enough to handle, but kind of weird. > >> 3. What on earth is IRQ_TYPE_NONE? That's an OpenPIC flag that just >> disables the interrupt entirely. Why would you want to configure >> things that way? > In Linux DT, the IRQ_TYPE_NONE is equivalent of our > pair -> don't change > actual interrupt configuration, i want to reuse default or already > preset configuration (from uboot for example). It's not a Linux thing, it's from the OpenPIC spec. Probably it just means the interrupt isn't configurable, no that it matters. > >> (#1 is the only one that really matters here; the rest are personal >> curiosity) >> >> If the answer to (1) is "yes", there would be a good case for adding >> something like a ofw_bus_map_gpio_as_intr() or the like that takes the >> same arguments as ofw_bus_map_intr() and propagates them to the >> control in some way but with a flag. It's an easy extension and this >> is the reason the function has "ofw" in the name. My one qualm about >> that is that there is no guarantee that a GPIO controller can actually >> provide interrupts on a random GPIO (nor is it clear that that is >> actually what is wanted here). That qualm applies to both APIs, of >> course, and I still don't see a rationale for throwing away the >> existing code even if (1) is true. > But current INTRNG is no longer OFW centric, it can easily handle any > other 'configuration source'. We removed it (from INTRNG core) at > r297539, at request from MIPS folks. > But again, all what I want in this area is (for me) simple change of > your ofw_bus_map_intr() method to something like: > > enum intr_map_data_type { > INTR_MAP_DATA_OFW, > INTR_MAP_DATA_GPIO, > ... > }; > > int > bus_map_intr(device_t dev, enum intr_map_data_type type, uintptr_t > pic_id, void *config_data, int config_size) > > Please, don't take current implementation of bus_map_intr() in account. > From my current point of view, the method is badly named and his name > doesn't reflect its functionality. Fair enough. My core issue is basically that this code connects interrupts to the newbus hierarchy, which simply doesn't work in a lot of cases when the newbus hierarchy doesn't match the interrupt one, which is quite common (see my other mail). Unfortunately, that's a fundamental architectural issue with r301453 and so I'm not sure how to proceed here. The current mechanism I was discussing is not OFW-centric either, although it is true that the only current mapping method (ofw_bus_map_intr()) is (though it can be abused for non-OFW data sources successfully), but you could easily add an acpi_bus_map_intr() or whatever that takes different data and connects in the MD interrupt code. I have no objections to that at all. I'm still confused about how your GPIO interrupts work (above), but that's a detail. Is that something that you think would be workable? I'd be more than happy to write some prototype code. -Nathan > > Michal >> I've changed the mailing list here since the SVN list really isn't the >> best place. For people who want context and are seeing this for the >> first time, please see the other email I just sent. >> -Nathan From owner-freebsd-arch@freebsd.org Mon Jul 25 16:32:26 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 40254BA440D for ; Mon, 25 Jul 2016 16:32:26 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-it0-x22e.google.com (mail-it0-x22e.google.com [IPv6:2607:f8b0:4001:c0b::22e]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 0A07A12AB for ; Mon, 25 Jul 2016 16:32:26 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-it0-x22e.google.com with SMTP id f6so109809247ith.0 for ; Mon, 25 Jul 2016 09:32:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=QKCY80C8L+zXbQcVDxmPxDiW9bDkRFRvzcNBD1pN9Q4=; b=gWAo/Ot49q7I8eBMHdPZQIyK/1jCV2QHM2YnJ7Umiz1HMfuDhFCoXxnon39/4kJ6M5 +qG3cRl/vkO6jf6VC78lkh+qeGtExPLv/AR3sU/Dq7klYsF7UDNPh1mTQWbnJjl4CImA G1ZQ9SwatXhW1G1qlNXUVXqMZjOS5eqauJ4fmtjCOVLdy8gA8pHz0TXytK73WTy7QO7E gyJlwCRWXbNo8me8PVdN5mXUwCHyXskxp9RFTNUq+uFXlH9Olu2g1n2FwChZ/Z4RZyaq nol4rxcVmeAPt4rMHh+guel1nAG4tRAhxVJZDs2YjhmckWRHF3uvgvhhiV9w6aC3HR33 qAfw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=QKCY80C8L+zXbQcVDxmPxDiW9bDkRFRvzcNBD1pN9Q4=; b=bsjA9dRlFVBBAzTbDI+t1sDUtZWDZ8wt6BEqF8AjwZJQbIo9Oadc31+hYbDMJK26lL big82ja8AS3nqUjfTFEtgPJKeZ3l7/spFwsPsW7dPXkFePALNitDVFr9ELW0O6p0FgLl 9G1pHcSN3gvoA28/yrdfVRz4MkdijeTsd2SweRsz6oNTd2s1bcydAv+oihuCzrGeTxNj /T4W5szETVajJYBGi/VImN7+jubzpkGlPoC3BMGfAhC9srk1Ud7Fr6+wKM+Hxt584D8o irnSSJ7KNt4J+lf7aPN87u0b+hSSPTcnhCPQ/ea3p0A9NHWliOIPMB+HH7rNqmrKpWY/ BIMQ== X-Gm-Message-State: ALyK8tI6RwSpNxRsRAt51UTErmnf1ZfnwDNAAkuyGA885mqQ0f0y8dYUH676cycSPPsys8XxbGwDhILfVNxBjg== X-Received: by 10.36.66.2 with SMTP id i2mr40710179itb.53.1469464345219; Mon, 25 Jul 2016 09:32:25 -0700 (PDT) MIME-Version: 1.0 Sender: wlosh@bsdimp.com Received: by 10.79.137.131 with HTTP; Mon, 25 Jul 2016 09:32:24 -0700 (PDT) X-Originating-IP: [50.205.115.50] In-Reply-To: References: <201606051620.u55GKD5S066398@repo.freebsd.org> <578E0B5D.3070105@FreeBSD.org> <578F6075.7010500@FreeBSD.org> <05a80ac6-4285-ec9d-36e9-2f92c609f746@freebsd.org> <57907B0F.9070204@FreeBSD.org> <9d2a224c-b787-2875-5984-a7a2354e8695@freebsd.org> <57934ABD.6010807@FreeBSD.org> <4e7a3e8f-cc21-f5f2-e3e0-4dbd554a4cd0@freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <57961549.4020105@FreeBSD.org> From: Warner Losh Date: Mon, 25 Jul 2016 10:32:24 -0600 X-Google-Sender-Auth: puGqSw5KkQDAcsGN8wUg4sd6mPw Message-ID: Subject: Re: INTRNG (Was: svn commit: r301453....) To: Nathan Whitehorn Cc: Michal Meloun , Svatopluk Kraus , "freebsd-arch@freebsd.org" , "freebsd-arm@freebsd.org" Content-Type: text/plain; charset=UTF-8 X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 Jul 2016 16:32:26 -0000 On Mon, Jul 25, 2016 at 8:05 AM, Nathan Whitehorn wrote: > That wasn't my question. Are these particular device drivers allocating > interrupts both on the GPIOs in their "interrupts" property (which are > entirely GPIOs in this example) *and* on the GPIOs listed as resources but > not listed as interrupts? If they are, then you need a switching mechanism, > but that seems pretty unlikely given the names of the non-interrupt GPIOs > (they look like outputs). It would also be a somewhat deranged way to set up > a device tree -- not that that rules it out or anything. On Atmel, there's a situation that this covers, I think. The MCI device has an interrupt in the core: mmc0: mmc@fffa8000 { compatible = "atmel,hsmci"; reg = <0xfffa8000 0x600>; interrupts = <9 IRQ_TYPE_LEVEL_HIGH 0>; #address-cells = <1>; #size-cells = <0>; pinctrl-names = "default"; clocks = <&mci0_clk>; clock-names = "mci_clk"; status = "disabled"; }; and in other places wires in GPIO interrupts for things like card eject / insertion. mmc0: mmc@f0008000 { pinctrl-0 = < &pinctrl_board_mmc0 &pinctrl_mmc0_slot0_clk_cmd_dat0 &pinctrl_mmc0_slot0_dat1_3>; status = "okay"; slot@0 { reg = <0>; bus-width = <4>; cd-gpios = <&pioD 15 GPIO_ACTIVE_HIGH>; }; }; an interrupt is registered on the cd-gpios pin for when the card changes. At least in linux, FreeBSD doesn't (yet) implement this, but will someday if I get back to the armv6 atmel work I started (see at91-cosino.dts for example, there's others). I think this is an example of what you are asking about, or did I get lost in the twisty maze of conversation zigs and zags... Warner From owner-freebsd-arch@freebsd.org Mon Jul 25 16:48:33 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id C15FCBA46BF; Mon, 25 Jul 2016 16:48:33 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from c.mail.sonic.net (c.mail.sonic.net [64.142.111.80]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id A8EDD189C; Mon, 25 Jul 2016 16:48:33 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from aurora.physics.berkeley.edu (aurora.physics.berkeley.edu [128.32.117.67]) (authenticated bits=0) by c.mail.sonic.net (8.15.1/8.15.1) with ESMTPSA id u6PGmOF8026555 (version=TLSv1.2 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Mon, 25 Jul 2016 09:48:25 -0700 Subject: Re: INTRNG (Was: svn commit: r301453....) To: Warner Losh References: <201606051620.u55GKD5S066398@repo.freebsd.org> <578E0B5D.3070105@FreeBSD.org> <578F6075.7010500@FreeBSD.org> <05a80ac6-4285-ec9d-36e9-2f92c609f746@freebsd.org> <57907B0F.9070204@FreeBSD.org> <9d2a224c-b787-2875-5984-a7a2354e8695@freebsd.org> <57934ABD.6010807@FreeBSD.org> <4e7a3e8f-cc21-f5f2-e3e0-4dbd554a4cd0@freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <57961549.4020105@FreeBSD.org> Cc: Michal Meloun , Svatopluk Kraus , "freebsd-arch@freebsd.org" , "freebsd-arm@freebsd.org" From: Nathan Whitehorn Message-ID: Date: Mon, 25 Jul 2016 09:48:24 -0700 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:45.0) Gecko/20100101 Thunderbird/45.1.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Sonic-CAuth: UmFuZG9tSVaSaSSOA20ymKx7VTUGGfy3r3xuPzV4lk/S++AVRaPyu3KE3BCJ3ljiLPFGLXxQ8TdHwi6TjuvbDozWsZQRiTuTHu0iM9dKFmc= X-Sonic-ID: C;6riHmodS5hG61ZtMTlz00w== M;1gbQmodS5hG61ZtMTlz00w== X-Spam-Flag: No X-Sonic-Spam-Details: 0.0/5.0 by cerberusd X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 Jul 2016 16:48:33 -0000 On 07/25/16 09:32, Warner Losh wrote: > On Mon, Jul 25, 2016 at 8:05 AM, Nathan Whitehorn > wrote: >> That wasn't my question. Are these particular device drivers allocating >> interrupts both on the GPIOs in their "interrupts" property (which are >> entirely GPIOs in this example) *and* on the GPIOs listed as resources but >> not listed as interrupts? If they are, then you need a switching mechanism, >> but that seems pretty unlikely given the names of the non-interrupt GPIOs >> (they look like outputs). It would also be a somewhat deranged way to set up >> a device tree -- not that that rules it out or anything. > On Atmel, there's a situation that this covers, I think. > > The MCI device has an interrupt in the core: > > mmc0: mmc@fffa8000 { > compatible = "atmel,hsmci"; > reg = <0xfffa8000 0x600>; > interrupts = <9 IRQ_TYPE_LEVEL_HIGH 0>; > #address-cells = <1>; > #size-cells = <0>; > pinctrl-names = "default"; > clocks = <&mci0_clk>; > clock-names = "mci_clk"; > status = "disabled"; > }; > > and in other places wires in GPIO interrupts for things like card > eject / insertion. > > mmc0: mmc@f0008000 { > pinctrl-0 = < > &pinctrl_board_mmc0 > &pinctrl_mmc0_slot0_clk_cmd_dat0 > &pinctrl_mmc0_slot0_dat1_3>; > status = "okay"; > slot@0 { > reg = <0>; > bus-width = <4>; > cd-gpios = <&pioD 15 GPIO_ACTIVE_HIGH>; > }; > }; > > an interrupt is registered on the cd-gpios pin for when the card changes. At > least in linux, FreeBSD doesn't (yet) implement this, but will someday if I get > back to the armv6 atmel work I started (see at91-cosino.dts for example, there's > others). > > I think this is an example of what you are asking about, or did I get > lost in the > twisty maze of conversation zigs and zags... > > Warner > Where we would run into (minor) problems is if the interrupt parent for the first mmc0 is the GPIO controller. More generally, if &pioD has interrupt children specified in some way that is not a tuple somewhere else in the tree then you would have to have methods to parse both interrupt specifiers as-obtained-from-interrupts-properties (or equivalent) and specifiers as-obtained-from-gpio-properties. If the tree picks one format and sticks with it, you can get away with just the one. Glancing through the DTS source for this board, that doesn't appear to be the case and the property formatting is uniform, but I might have missed something in one of the many #includes. As a general point, GPIO weirdness would be easy enough case to handle if it did come up (add some mapping method, as above) that I think we shouldn't worry too much about it from an architectural point of view. If a board appears that is set up this way, we can roll with the punches at that point and add whatever small amount of shim code that is required. It would be annoyance, sure, but not a real complication. -Nathan From owner-freebsd-arch@freebsd.org Tue Jul 26 04:24:55 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 70103BA56C5 for ; Tue, 26 Jul 2016 04:24:55 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-io0-x22a.google.com (mail-io0-x22a.google.com [IPv6:2607:f8b0:4001:c06::22a]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 24F3E17AA for ; Tue, 26 Jul 2016 04:24:55 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-io0-x22a.google.com with SMTP id b62so193568416iod.3 for ; Mon, 25 Jul 2016 21:24:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=WMB4q8qs9epdEO2K9XxEQLmKp/HlF7veu4NSwCqgS3U=; b=IAoGPYCX8NxORdQCCFHkalOvVRzQWsyBHLPQ5yU+x3cWuGc8MSZTpvkngNgM/5csN8 jypYrW3VC1bx0ZCftA7mKQlzdLxnwrNnmPtmzK+Eoa/vRq0leTga9Wp8CzvjvBl02/iN faocJAY0Wiqu+tGl+MyCphRW90j30AuWU3I1WMoMg0DrPr/lhnHmvIr8Egp1ZxDZwpfN Buf2N/SD4CNuJpSMsy4uxzO4OVBuyKSSqo+VBisLZxPqE4pJI6tQZLOvzZfLrcGVuh2J I77V8quAkfKFhZx+8ngKofXDpWEymdSGOhhWzYDW+L5JgMpRGBi14/4F86to2jdoh9OP QE4w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=WMB4q8qs9epdEO2K9XxEQLmKp/HlF7veu4NSwCqgS3U=; b=Uv2Dm3eRSA9oQ7t2p0cktDqxFjSon6eBq31mK2yz39/aTtGCqV4JGksLdGF/HV8WT+ UfG0bZPe/aS6K3MCdjDSl6ax0rHiOZKGu622EimTVYN7UTkmebbV10uZ/pPxYuLd0/lX uh/szY6B4Nb/3st+ZkXbaCpXWmqArNPBpSdHBIyIqdx0qwYcdcR9lEhRvLVDHAdIQpQX UxtwNeyapCL4R/j+CrD/ZLvcwHGwLBQJVCfhpt4o3S5t6NdCCidX2I90KqhCIHC++qtM yu1iejkrQ8qdJWcxqGwV2bai5Ywnk5XXacTr0g3Gh/ts9/smUHmrBPRntkQXKHHuA0SC TCQQ== X-Gm-Message-State: AEkoousDmIfa19iXDyeG5SUvYd+UPrJmCMW5WeGyWma+hKJxahvj0xlOqLa3JaaIX64arkQAl05pER2PVBQDqA== X-Received: by 10.107.133.93 with SMTP id h90mr23629391iod.16.1469507094415; Mon, 25 Jul 2016 21:24:54 -0700 (PDT) MIME-Version: 1.0 Sender: wlosh@bsdimp.com Received: by 10.79.137.131 with HTTP; Mon, 25 Jul 2016 21:24:53 -0700 (PDT) X-Originating-IP: [69.53.245.200] In-Reply-To: References: <201606051620.u55GKD5S066398@repo.freebsd.org> <578E0B5D.3070105@FreeBSD.org> <578F6075.7010500@FreeBSD.org> <05a80ac6-4285-ec9d-36e9-2f92c609f746@freebsd.org> <57907B0F.9070204@FreeBSD.org> <9d2a224c-b787-2875-5984-a7a2354e8695@freebsd.org> <57934ABD.6010807@FreeBSD.org> <4e7a3e8f-cc21-f5f2-e3e0-4dbd554a4cd0@freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <57961549.4020105@FreeBSD.org> From: Warner Losh Date: Mon, 25 Jul 2016 22:24:53 -0600 X-Google-Sender-Auth: kzVHCycSi51vnhDA-XUlIEAYMX0 Message-ID: Subject: Re: INTRNG (Was: svn commit: r301453....) To: Nathan Whitehorn Cc: Michal Meloun , Svatopluk Kraus , "freebsd-arch@freebsd.org" , "freebsd-arm@freebsd.org" Content-Type: text/plain; charset=UTF-8 X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Jul 2016 04:24:55 -0000 On Mon, Jul 25, 2016 at 10:48 AM, Nathan Whitehorn wrote: > > > On 07/25/16 09:32, Warner Losh wrote: >> >> On Mon, Jul 25, 2016 at 8:05 AM, Nathan Whitehorn >> wrote: >>> >>> That wasn't my question. Are these particular device drivers allocating >>> interrupts both on the GPIOs in their "interrupts" property (which are >>> entirely GPIOs in this example) *and* on the GPIOs listed as resources >>> but >>> not listed as interrupts? If they are, then you need a switching >>> mechanism, >>> but that seems pretty unlikely given the names of the non-interrupt GPIOs >>> (they look like outputs). It would also be a somewhat deranged way to set >>> up >>> a device tree -- not that that rules it out or anything. >> >> On Atmel, there's a situation that this covers, I think. >> >> The MCI device has an interrupt in the core: >> >> mmc0: mmc@fffa8000 { >> compatible = "atmel,hsmci"; >> reg = <0xfffa8000 0x600>; >> interrupts = <9 IRQ_TYPE_LEVEL_HIGH 0>; >> #address-cells = <1>; >> #size-cells = <0>; >> pinctrl-names = "default"; >> clocks = <&mci0_clk>; >> clock-names = "mci_clk"; >> status = "disabled"; >> }; >> >> and in other places wires in GPIO interrupts for things like card >> eject / insertion. >> >> mmc0: mmc@f0008000 { >> pinctrl-0 = < >> &pinctrl_board_mmc0 >> &pinctrl_mmc0_slot0_clk_cmd_dat0 >> &pinctrl_mmc0_slot0_dat1_3>; >> status = "okay"; >> slot@0 { >> reg = <0>; >> bus-width = <4>; >> cd-gpios = <&pioD 15 >> GPIO_ACTIVE_HIGH>; >> }; >> }; >> >> an interrupt is registered on the cd-gpios pin for when the card changes. >> At >> least in linux, FreeBSD doesn't (yet) implement this, but will someday if >> I get >> back to the armv6 atmel work I started (see at91-cosino.dts for example, >> there's >> others). >> >> I think this is an example of what you are asking about, or did I get >> lost in the >> twisty maze of conversation zigs and zags... >> >> Warner >> > > Where we would run into (minor) problems is if the interrupt parent for the > first mmc0 is the GPIO controller. More generally, if &pioD has interrupt > children specified in some way that is not a > tuple somewhere else in the tree then you would have to have methods to > parse both interrupt specifiers as-obtained-from-interrupts-properties (or > equivalent) and specifiers as-obtained-from-gpio-properties. If the tree > picks one format and sticks with it, you can get away with just the one. > Glancing through the DTS source for this board, that doesn't appear to be > the case and the property formatting is uniform, but I might have missed > something in one of the many #includes. Interrupts and GPIO specifiers are different in subtle ways. The interrupt parent for mmc0 is an AIC, which is also the ultimate parent of the GPIO controller. But the properties for the GPIO pins that act as interrupts and the interrupt specifiers are different. > As a general point, GPIO weirdness would be easy enough case to handle if it > did come up (add some mapping method, as above) that I think we shouldn't > worry too much about it from an architectural point of view. If a board > appears that is set up this way, we can roll with the punches at that point > and add whatever small amount of shim code that is required. It would be > annoyance, sure, but not a real complication. I suspect that either I don't understand the issue, or we'll have such boards very quickly. The Atmel design is fairly clean in comparison to other franken-horrors I've seen... Warner From owner-freebsd-arch@freebsd.org Tue Jul 26 05:41:44 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 4AD2CBA5518; Tue, 26 Jul 2016 05:41:44 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from c.mail.sonic.net (c.mail.sonic.net [64.142.111.80]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 30F731A79; Tue, 26 Jul 2016 05:41:43 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from zeppelin.tachypleus.net (75-101-50-44.static.sonic.net [75.101.50.44]) (authenticated bits=0) by c.mail.sonic.net (8.15.1/8.15.1) with ESMTPSA id u6Q5fe9B016441 (version=TLSv1.2 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Mon, 25 Jul 2016 22:41:40 -0700 Subject: Re: INTRNG (Was: svn commit: r301453....) To: Warner Losh References: <201606051620.u55GKD5S066398@repo.freebsd.org> <578E0B5D.3070105@FreeBSD.org> <578F6075.7010500@FreeBSD.org> <05a80ac6-4285-ec9d-36e9-2f92c609f746@freebsd.org> <57907B0F.9070204@FreeBSD.org> <9d2a224c-b787-2875-5984-a7a2354e8695@freebsd.org> <57934ABD.6010807@FreeBSD.org> <4e7a3e8f-cc21-f5f2-e3e0-4dbd554a4cd0@freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <57961549.4020105@FreeBSD.org> Cc: Michal Meloun , Svatopluk Kraus , "freebsd-arch@freebsd.org" , "freebsd-arm@freebsd.org" From: Nathan Whitehorn Message-ID: Date: Mon, 25 Jul 2016 22:41:40 -0700 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:45.0) Gecko/20100101 Thunderbird/45.1.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Sonic-CAuth: UmFuZG9tSVbpDTl1BNmgl0Wr+5ocv7kfz684yOseer/bHoSJHwnOFcZHg91wGuPzUq0zW1vRIkGotifzkeSe0H6/7tEmjKhsI5DWIe9oNNI= X-Sonic-ID: C;XKtWoPNS5hGVlZtMTlz00w== M;6Ji+oPNS5hGVlZtMTlz00w== X-Spam-Flag: No X-Sonic-Spam-Details: 0.0/5.0 by cerberusd X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Jul 2016 05:41:44 -0000 On 07/25/16 21:24, Warner Losh wrote: > On Mon, Jul 25, 2016 at 10:48 AM, Nathan Whitehorn > wrote: >> >> On 07/25/16 09:32, Warner Losh wrote: >>> On Mon, Jul 25, 2016 at 8:05 AM, Nathan Whitehorn >>> wrote: >>>> That wasn't my question. Are these particular device drivers allocating >>>> interrupts both on the GPIOs in their "interrupts" property (which are >>>> entirely GPIOs in this example) *and* on the GPIOs listed as resources >>>> but >>>> not listed as interrupts? If they are, then you need a switching >>>> mechanism, >>>> but that seems pretty unlikely given the names of the non-interrupt GPIOs >>>> (they look like outputs). It would also be a somewhat deranged way to set >>>> up >>>> a device tree -- not that that rules it out or anything. >>> On Atmel, there's a situation that this covers, I think. >>> >>> The MCI device has an interrupt in the core: >>> >>> mmc0: mmc@fffa8000 { >>> compatible = "atmel,hsmci"; >>> reg = <0xfffa8000 0x600>; >>> interrupts = <9 IRQ_TYPE_LEVEL_HIGH 0>; >>> #address-cells = <1>; >>> #size-cells = <0>; >>> pinctrl-names = "default"; >>> clocks = <&mci0_clk>; >>> clock-names = "mci_clk"; >>> status = "disabled"; >>> }; >>> >>> and in other places wires in GPIO interrupts for things like card >>> eject / insertion. >>> >>> mmc0: mmc@f0008000 { >>> pinctrl-0 = < >>> &pinctrl_board_mmc0 >>> &pinctrl_mmc0_slot0_clk_cmd_dat0 >>> &pinctrl_mmc0_slot0_dat1_3>; >>> status = "okay"; >>> slot@0 { >>> reg = <0>; >>> bus-width = <4>; >>> cd-gpios = <&pioD 15 >>> GPIO_ACTIVE_HIGH>; >>> }; >>> }; >>> >>> an interrupt is registered on the cd-gpios pin for when the card changes. >>> At >>> least in linux, FreeBSD doesn't (yet) implement this, but will someday if >>> I get >>> back to the armv6 atmel work I started (see at91-cosino.dts for example, >>> there's >>> others). >>> >>> I think this is an example of what you are asking about, or did I get >>> lost in the >>> twisty maze of conversation zigs and zags... >>> >>> Warner >>> >> Where we would run into (minor) problems is if the interrupt parent for the >> first mmc0 is the GPIO controller. More generally, if &pioD has interrupt >> children specified in some way that is not a >> tuple somewhere else in the tree then you would have to have methods to >> parse both interrupt specifiers as-obtained-from-interrupts-properties (or >> equivalent) and specifiers as-obtained-from-gpio-properties. If the tree >> picks one format and sticks with it, you can get away with just the one. >> Glancing through the DTS source for this board, that doesn't appear to be >> the case and the property formatting is uniform, but I might have missed >> something in one of the many #includes. > Interrupts and GPIO specifiers are different in subtle ways. The interrupt > parent for mmc0 is an AIC, which is also the ultimate parent of the GPIO > controller. That is what it looked like. > But the properties for the GPIO pins that act as interrupts and > the interrupt specifiers are different. So there are devices with both interrupts = , #interrupt-parent = <&gpio> and gpios = <&gpio bleh baz> where "Bleh baz" is formatted different than "foo bar" and both are meant to be treated as interrupts? It's fine if there are, but I haven't seen any such device trees yet. >> As a general point, GPIO weirdness would be easy enough case to handle if it >> did come up (add some mapping method, as above) that I think we shouldn't >> worry too much about it from an architectural point of view. If a board >> appears that is set up this way, we can roll with the punches at that point >> and add whatever small amount of shim code that is required. It would be >> annoyance, sure, but not a real complication. > I suspect that either I don't understand the issue, or we'll have such boards > very quickly. The Atmel design is fairly clean in comparison to other > franken-horrors > I've seen... People do weird things for sure. My point is just that the details of how (implicit) GPIO interrupts are formatted just isn't that important. It's easy enough to add special code for devices that are set up in bizarre ways as they are spotted in the wild and that special code is so minor that it doesn't matter for the design of the API. This is a point I was just curious about, since I had never actually seen device trees set up that way. The issue with this patch is, at its core, whether you have an architecture that relies on the newbus hierarchy (like r301453) or that allows links outside of that hierarchy that can cross branches or go the "wrong" way, like the previously existing code. There are some other differences, of varying degrees of importance, but that's the really fundamental one. I haven't seen any cases where r301453 provides functionality absent in the already existing system, but there seem to be a large number of cases (see the first email I sent to freebsd-arch or https://wiki.freebsd.org/Complicated_Interrupts) that the API in r301453 cannot accommodate and that are needed to support a variety of hardware. Since having two APIs will be a significant maintenance burden, if there are cases the existing code can't handle, I would like to know about them; otherwise, I think we should back out r301453 and stick to the standard device tree interrupt mapping mechanism on ARM instead. I'm happy to help implement any necessary enhancements there (for example, dealing with weirdly-encoded GPIOs). -Nathan > > Warner > From owner-freebsd-arch@freebsd.org Tue Jul 26 13:41:04 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 59B47BA593C; Tue, 26 Jul 2016 13:41:04 +0000 (UTC) (envelope-from mmel@FreeBSD.org) Received: from mail.miracle.cz (mail.miracle.cz [193.84.128.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mail.miracle.cz", Issuer "Miracle Group Root CA" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 147FA1A92; Tue, 26 Jul 2016 13:41:03 +0000 (UTC) (envelope-from mmel@FreeBSD.org) Received: from [193.84.128.50] (meloun.ad.miracle.cz [193.84.128.50]) by mail.miracle.cz (Postfix) with ESMTPSA id C93E63AC81; Tue, 26 Jul 2016 15:40:55 +0200 (CEST) Subject: Re: INTRNG (Was: svn commit: r301453....) To: Nathan Whitehorn References: <201606051620.u55GKD5S066398@repo.freebsd.org> <578E0B5D.3070105@FreeBSD.org> <578F6075.7010500@FreeBSD.org> <05a80ac6-4285-ec9d-36e9-2f92c609f746@freebsd.org> <57907B0F.9070204@FreeBSD.org> <9d2a224c-b787-2875-5984-a7a2354e8695@freebsd.org> <57934ABD.6010807@FreeBSD.org> <4e7a3e8f-cc21-f5f2-e3e0-4dbd554a4cd0@freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <57961549.4020105@FreeBSD.org> Cc: Svatopluk Kraus , "freebsd-arch@freebsd.org" , "freebsd-arm@freebsd.org" From: Michal Meloun X-Enigmail-Draft-Status: N1110 Message-ID: <57976867.6080705@FreeBSD.org> Date: Tue, 26 Jul 2016 15:40:55 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.4.3 (mail.miracle.cz); Tue, 26 Jul 2016 15:40:55 +0200 (CEST) X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Jul 2016 13:41:04 -0000 Dne 26.07.2016 v 7:41 Nathan Whitehorn napsal(a): > > > On 07/25/16 21:24, Warner Losh wrote: >> On Mon, Jul 25, 2016 at 10:48 AM, Nathan Whitehorn >> wrote: >>> >>> On 07/25/16 09:32, Warner Losh wrote: >>>> On Mon, Jul 25, 2016 at 8:05 AM, Nathan Whitehorn >>>> wrote: >>>>> That wasn't my question. Are these particular device drivers >>>>> allocating >>>>> interrupts both on the GPIOs in their "interrupts" property (which >>>>> are >>>>> entirely GPIOs in this example) *and* on the GPIOs listed as >>>>> resources >>>>> but >>>>> not listed as interrupts? If they are, then you need a switching >>>>> mechanism, >>>>> but that seems pretty unlikely given the names of the >>>>> non-interrupt GPIOs >>>>> (they look like outputs). It would also be a somewhat deranged way >>>>> to set >>>>> up >>>>> a device tree -- not that that rules it out or anything. >>>> On Atmel, there's a situation that this covers, I think. >>>> >>>> The MCI device has an interrupt in the core: >>>> >>>> mmc0: mmc@fffa8000 { >>>> compatible = "atmel,hsmci"; >>>> reg = <0xfffa8000 0x600>; >>>> interrupts = <9 >>>> IRQ_TYPE_LEVEL_HIGH 0>; >>>> #address-cells = <1>; >>>> #size-cells = <0>; >>>> pinctrl-names = "default"; >>>> clocks = <&mci0_clk>; >>>> clock-names = "mci_clk"; >>>> status = "disabled"; >>>> }; >>>> >>>> and in other places wires in GPIO interrupts for things like card >>>> eject / insertion. >>>> >>>> mmc0: mmc@f0008000 { >>>> pinctrl-0 = < >>>> &pinctrl_board_mmc0 >>>> >>>> &pinctrl_mmc0_slot0_clk_cmd_dat0 >>>> &pinctrl_mmc0_slot0_dat1_3>; >>>> status = "okay"; >>>> slot@0 { >>>> reg = <0>; >>>> bus-width = <4>; >>>> cd-gpios = <&pioD 15 >>>> GPIO_ACTIVE_HIGH>; >>>> }; >>>> }; >>>> >>>> an interrupt is registered on the cd-gpios pin for when the card >>>> changes. >>>> At >>>> least in linux, FreeBSD doesn't (yet) implement this, but will >>>> someday if >>>> I get >>>> back to the armv6 atmel work I started (see at91-cosino.dts for >>>> example, >>>> there's >>>> others). >>>> >>>> I think this is an example of what you are asking about, or did I get >>>> lost in the >>>> twisty maze of conversation zigs and zags... >>>> >>>> Warner >>>> >>> Where we would run into (minor) problems is if the interrupt parent >>> for the >>> first mmc0 is the GPIO controller. More generally, if &pioD has >>> interrupt >>> children specified in some way that is not a >> high/whatever> >>> tuple somewhere else in the tree then you would have to have methods to >>> parse both interrupt specifiers >>> as-obtained-from-interrupts-properties (or >>> equivalent) and specifiers as-obtained-from-gpio-properties. If the >>> tree >>> picks one format and sticks with it, you can get away with just the >>> one. >>> Glancing through the DTS source for this board, that doesn't appear >>> to be >>> the case and the property formatting is uniform, but I might have >>> missed >>> something in one of the many #includes. >> Interrupts and GPIO specifiers are different in subtle ways. The >> interrupt >> parent for mmc0 is an AIC, which is also the ultimate parent of the GPIO >> controller. > > That is what it looked like. > >> But the properties for the GPIO pins that act as interrupts and >> the interrupt specifiers are different. > > So there are devices with both interrupts = , > #interrupt-parent = <&gpio> and gpios = <&gpio bleh baz> where "Bleh > baz" is formatted different than "foo bar" and both are meant to be > treated as interrupts? > > It's fine if there are, but I haven't seen any such device trees yet. I think that yes, but I'm not ready to search all 1372 Linux DT files only for explicit example. But your question is invalid from beginning. 1) Format of each single 'interrupts' and '*-gpios' property is different because each single one are defined differently. 2) The question should not be 'Are there device' but 'Is this combination valid' Moreover, I'm curious why you want OFW property for gpio interrupts at all, given gpio can be instantiated by other entity (PCIe, USB). Please see: https://svnweb.freebsd.org/base/head/sys/dev/gpio/gpiobus.c?revision=301539&view=markup#l92 and, for example this one controller https://svnweb.freebsd.org/base/head/sys/arm/nvidia/tegra_gpio.c?revision=300149&view=markup#l575 > >>> As a general point, GPIO weirdness would be easy enough case to >>> handle if it >>> did come up (add some mapping method, as above) that I think we >>> shouldn't >>> worry too much about it from an architectural point of view. If a board >>> appears that is set up this way, we can roll with the punches at >>> that point >>> and add whatever small amount of shim code that is required. It >>> would be >>> annoyance, sure, but not a real complication. >> I suspect that either I don't understand the issue, or we'll have >> such boards >> very quickly. The Atmel design is fairly clean in comparison to other >> franken-horrors >> I've seen... > > People do weird things for sure. My point is just that the details of > how (implicit) GPIO interrupts are formatted just isn't that > important. It's easy enough to add special code for devices that are > set up in bizarre ways as they are spotted in the wild and that > special code is so minor that it doesn't matter for the design of the > API. This is a point I was just curious about, since I had never > actually seen device trees set up that way. > > The issue with this patch is, at its core, whether you have an > architecture that relies on the newbus hierarchy (like r301453) or > that allows links outside of that hierarchy that can cross branches or > go the "wrong" way, like the previously existing code. OK. The r301453 : - it removes unnecessary idea of virtual IRQs. I gave you some examples where virtual IRQ concept fails. Also, both variants needs attached PIC at bus_alloc_resource() time, so timing wasn't been changed. - it implements new BUS_MAP_INTR(). As I understand it, this is problematic for you, and I'm ready to change it. But I need more details than "it's fundamentally broken". > There are some other differences, of varying degrees of importance, > but that's the really fundamental one. I haven't seen any cases where > r301453 provides functionality absent in the already existing system, > but there seem to be a large number of cases (see the first email I > sent to freebsd-arch or > https://wiki.freebsd.org/Complicated_Interrupts) that the API in > r301453 cannot accommodate and that are needed to support a variety of > hardware. Which single feature has been removed by r301453? Michal > Since having two APIs will be a significant maintenance burden, if > there are cases the existing code can't handle, I would like to know > about them; otherwise, I think we should back out r301453 and stick to > the standard device tree interrupt mapping mechanism on ARM instead. > I'm happy to help implement any necessary enhancements there (for > example, dealing with weirdly-encoded GPIOs). > -Nathan > >> >> Warner >> > From owner-freebsd-arch@freebsd.org Tue Jul 26 14:56:13 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 3E535BA4EE5; Tue, 26 Jul 2016 14:56:13 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from c.mail.sonic.net (c.mail.sonic.net [64.142.111.80]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 1FD6515C6; Tue, 26 Jul 2016 14:56:12 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from zeppelin.tachypleus.net (75-101-50-44.static.sonic.net [75.101.50.44]) (authenticated bits=0) by c.mail.sonic.net (8.15.1/8.15.1) with ESMTPSA id u6QEu9ln012679 (version=TLSv1.2 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Tue, 26 Jul 2016 07:56:10 -0700 Subject: Re: INTRNG (Was: svn commit: r301453....) To: Michal Meloun References: <201606051620.u55GKD5S066398@repo.freebsd.org> <578E0B5D.3070105@FreeBSD.org> <578F6075.7010500@FreeBSD.org> <05a80ac6-4285-ec9d-36e9-2f92c609f746@freebsd.org> <57907B0F.9070204@FreeBSD.org> <9d2a224c-b787-2875-5984-a7a2354e8695@freebsd.org> <57934ABD.6010807@FreeBSD.org> <4e7a3e8f-cc21-f5f2-e3e0-4dbd554a4cd0@freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <57961549.4020105@FreeBSD.org> <57976867.6080705@FreeBSD.org> Cc: Svatopluk Kraus , "freebsd-arch@freebsd.org" , "freebsd-arm@freebsd.org" From: Nathan Whitehorn Message-ID: Date: Tue, 26 Jul 2016 07:56:09 -0700 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:45.0) Gecko/20100101 Thunderbird/45.1.0 MIME-Version: 1.0 In-Reply-To: <57976867.6080705@FreeBSD.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Sonic-CAuth: UmFuZG9tSVaniCWrbJX6qqHh22bYhEhWifzY8aePNVKjBopjasWF9M3oXmzYWVnt0Cr9bHpsUzjkB2nOfrKUTUEkOrKfAyUbcNU17NW4YS0= X-Sonic-ID: C;PlpNFkFT5hGHx5tMTlz00w== M;4ry0FkFT5hGHx5tMTlz00w== X-Spam-Flag: No X-Sonic-Spam-Details: 0.0/5.0 by cerberusd X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Jul 2016 14:56:13 -0000 On 07/26/16 06:40, Michal Meloun wrote: > Dne 26.07.2016 v 7:41 Nathan Whitehorn napsal(a): >> >> On 07/25/16 21:24, Warner Losh wrote: >>> On Mon, Jul 25, 2016 at 10:48 AM, Nathan Whitehorn >>> wrote: >>>> On 07/25/16 09:32, Warner Losh wrote: >>>>> On Mon, Jul 25, 2016 at 8:05 AM, Nathan Whitehorn >>>>> wrote: >>>>>> That wasn't my question. Are these particular device drivers >>>>>> allocating >>>>>> interrupts both on the GPIOs in their "interrupts" property (which >>>>>> are >>>>>> entirely GPIOs in this example) *and* on the GPIOs listed as >>>>>> resources >>>>>> but >>>>>> not listed as interrupts? If they are, then you need a switching >>>>>> mechanism, >>>>>> but that seems pretty unlikely given the names of the >>>>>> non-interrupt GPIOs >>>>>> (they look like outputs). It would also be a somewhat deranged way >>>>>> to set >>>>>> up >>>>>> a device tree -- not that that rules it out or anything. >>>>> On Atmel, there's a situation that this covers, I think. >>>>> >>>>> The MCI device has an interrupt in the core: >>>>> >>>>> mmc0: mmc@fffa8000 { >>>>> compatible = "atmel,hsmci"; >>>>> reg = <0xfffa8000 0x600>; >>>>> interrupts = <9 >>>>> IRQ_TYPE_LEVEL_HIGH 0>; >>>>> #address-cells = <1>; >>>>> #size-cells = <0>; >>>>> pinctrl-names = "default"; >>>>> clocks = <&mci0_clk>; >>>>> clock-names = "mci_clk"; >>>>> status = "disabled"; >>>>> }; >>>>> >>>>> and in other places wires in GPIO interrupts for things like card >>>>> eject / insertion. >>>>> >>>>> mmc0: mmc@f0008000 { >>>>> pinctrl-0 = < >>>>> &pinctrl_board_mmc0 >>>>> >>>>> &pinctrl_mmc0_slot0_clk_cmd_dat0 >>>>> &pinctrl_mmc0_slot0_dat1_3>; >>>>> status = "okay"; >>>>> slot@0 { >>>>> reg = <0>; >>>>> bus-width = <4>; >>>>> cd-gpios = <&pioD 15 >>>>> GPIO_ACTIVE_HIGH>; >>>>> }; >>>>> }; >>>>> >>>>> an interrupt is registered on the cd-gpios pin for when the card >>>>> changes. >>>>> At >>>>> least in linux, FreeBSD doesn't (yet) implement this, but will >>>>> someday if >>>>> I get >>>>> back to the armv6 atmel work I started (see at91-cosino.dts for >>>>> example, >>>>> there's >>>>> others). >>>>> >>>>> I think this is an example of what you are asking about, or did I get >>>>> lost in the >>>>> twisty maze of conversation zigs and zags... >>>>> >>>>> Warner >>>>> >>>> Where we would run into (minor) problems is if the interrupt parent >>>> for the >>>> first mmc0 is the GPIO controller. More generally, if &pioD has >>>> interrupt >>>> children specified in some way that is not a >>> high/whatever> >>>> tuple somewhere else in the tree then you would have to have methods to >>>> parse both interrupt specifiers >>>> as-obtained-from-interrupts-properties (or >>>> equivalent) and specifiers as-obtained-from-gpio-properties. If the >>>> tree >>>> picks one format and sticks with it, you can get away with just the >>>> one. >>>> Glancing through the DTS source for this board, that doesn't appear >>>> to be >>>> the case and the property formatting is uniform, but I might have >>>> missed >>>> something in one of the many #includes. >>> Interrupts and GPIO specifiers are different in subtle ways. The >>> interrupt >>> parent for mmc0 is an AIC, which is also the ultimate parent of the GPIO >>> controller. >> That is what it looked like. >> >>> But the properties for the GPIO pins that act as interrupts and >>> the interrupt specifiers are different. >> So there are devices with both interrupts = , >> #interrupt-parent = <&gpio> and gpios = <&gpio bleh baz> where "Bleh >> baz" is formatted different than "foo bar" and both are meant to be >> treated as interrupts? >> >> It's fine if there are, but I haven't seen any such device trees yet. > I think that yes, but I'm not ready to search all 1372 Linux DT files > only for explicit example. > But your question is invalid from beginning. > 1) Format of each single 'interrupts' and '*-gpios' property is > different because each single one are defined differently. > 2) The question should not be 'Are there device' but 'Is this > combination valid' > > Moreover, I'm curious why you want OFW property for gpio interrupts at all, > given gpio can be instantiated by other entity (PCIe, USB). > > Please see: > https://svnweb.freebsd.org/base/head/sys/dev/gpio/gpiobus.c?revision=301539&view=markup#l92 > and, for example this one controller > https://svnweb.freebsd.org/base/head/sys/arm/nvidia/tegra_gpio.c?revision=300149&view=markup#l575 Well, it's part of the device tree standard, so dealing with GPIO properties seems reasonable where they occur. You could, of course, have systems without device trees and GPIOs. I'm not sure where you are going with this. > >>>> As a general point, GPIO weirdness would be easy enough case to >>>> handle if it >>>> did come up (add some mapping method, as above) that I think we >>>> shouldn't >>>> worry too much about it from an architectural point of view. If a board >>>> appears that is set up this way, we can roll with the punches at >>>> that point >>>> and add whatever small amount of shim code that is required. It >>>> would be >>>> annoyance, sure, but not a real complication. >>> I suspect that either I don't understand the issue, or we'll have >>> such boards >>> very quickly. The Atmel design is fairly clean in comparison to other >>> franken-horrors >>> I've seen... >> People do weird things for sure. My point is just that the details of >> how (implicit) GPIO interrupts are formatted just isn't that >> important. It's easy enough to add special code for devices that are >> set up in bizarre ways as they are spotted in the wild and that >> special code is so minor that it doesn't matter for the design of the >> API. This is a point I was just curious about, since I had never >> actually seen device trees set up that way. >> >> The issue with this patch is, at its core, whether you have an >> architecture that relies on the newbus hierarchy (like r301453) or >> that allows links outside of that hierarchy that can cross branches or >> go the "wrong" way, like the previously existing code. > OK. The r301453 : > - it removes unnecessary idea of virtual IRQs. I gave you some examples > where virtual IRQ concept fails. I have never seen any of these examples. The concept is *extremely* necessary, which is why both Linux and FreeBSD decided to use it independently There is no way to handle parent buses with a single rman and devices on multiple PICs with overlapping interrupt ranges without them; neither is there a way to decode arbitrary-length interrupt specifiers or to handle things like MSIs. Please see the list of cases at https://wiki.freebsd.org/Complicated_Interrupts and in my earlier email for some examples of things that you just can't represent with this new system, as far as I can tell. > Also, both variants needs attached PIC at bus_alloc_resource() time, > so timing wasn't been changed. They absolutely do not, as I have explained repeatedly. Due to parent devices with interrupts handled by their bus children, this is a hard requirement of any workable system. This is not a theoretical issue; I have lots of hardware like this. > > - it implements new BUS_MAP_INTR(). As I understand it, this is > problematic for you, and I'm ready to change it. But I need more details > than "it's fundamentally broken". Please explain (a) what cases it handles that the existing code and does, and (b) how you would resolve each of the cases on the wiki page I sent. The general issue is that it traces the newbus hierarchy, when interrupts often do not, and so breaks when you have links to as-yet-unattached parts of the hierarchy. It also relies on the assigned "IRQ" numbers being nonoverlapping to avoid rman errors, but that's a relatively minor thing. > >> There are some other differences, of varying degrees of importance, >> but that's the really fundamental one. I haven't seen any cases where >> r301453 provides functionality absent in the already existing system, >> but there seem to be a large number of cases (see the first email I >> sent to freebsd-arch or >> https://wiki.freebsd.org/Complicated_Interrupts) that the API in >> r301453 cannot accommodate and that are needed to support a variety of >> hardware. > Which single feature has been removed by r301453? Nothing has been removed by it, because the normal code is intact. However, the new code is less functional than the old code, so cannot replace it. In particular, it is architecturally incapable of working with the kinds of device trees found on PowerPC systems (see the wiki page). That means we would have to keep both indefinitely, which is a significant maintenance burden to no gain whatsoever. -Nathan > Michal > >> Since having two APIs will be a significant maintenance burden, if >> there are cases the existing code can't handle, I would like to know >> about them; otherwise, I think we should back out r301453 and stick to >> the standard device tree interrupt mapping mechanism on ARM instead. >> I'm happy to help implement any necessary enhancements there (for >> example, dealing with weirdly-encoded GPIOs). >> -Nathan >> >>> Warner >>> From owner-freebsd-arch@freebsd.org Wed Jul 27 16:27:58 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id A7F47BA6EEB; Wed, 27 Jul 2016 16:27:58 +0000 (UTC) (envelope-from mmel@FreeBSD.org) Received: from mail.miracle.cz (mail.miracle.cz [193.84.128.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mail.miracle.cz", Issuer "Miracle Group Root CA" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 42F7A1957; Wed, 27 Jul 2016 16:27:57 +0000 (UTC) (envelope-from mmel@FreeBSD.org) Received: from [193.84.128.50] (meloun.ad.miracle.cz [193.84.128.50]) by mail.miracle.cz (Postfix) with ESMTPSA id 3A17C3AC87; Wed, 27 Jul 2016 18:27:49 +0200 (CEST) Subject: Re: INTRNG (Was: svn commit: r301453....) To: Nathan Whitehorn References: <201606051620.u55GKD5S066398@repo.freebsd.org> <578E0B5D.3070105@FreeBSD.org> <578F6075.7010500@FreeBSD.org> <05a80ac6-4285-ec9d-36e9-2f92c609f746@freebsd.org> <57907B0F.9070204@FreeBSD.org> <9d2a224c-b787-2875-5984-a7a2354e8695@freebsd.org> <57934ABD.6010807@FreeBSD.org> <4e7a3e8f-cc21-f5f2-e3e0-4dbd554a4cd0@freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <57961549.4020105@FreeBSD.org> <57976867.6080705@FreeBSD.org> Cc: Svatopluk Kraus , "freebsd-arch@freebsd.org" , "freebsd-arm@freebsd.org" From: Michal Meloun X-Enigmail-Draft-Status: N1110 Message-ID: <5798E104.5020104@FreeBSD.org> Date: Wed, 27 Jul 2016 18:27:48 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.4.3 (mail.miracle.cz); Wed, 27 Jul 2016 18:27:49 +0200 (CEST) Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.22 X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 27 Jul 2016 16:27:58 -0000 Dne 26.07.2016 v 16:56 Nathan Whitehorn napsal(a): > > > On 07/26/16 06:40, Michal Meloun wrote: >> Dne 26.07.2016 v 7:41 Nathan Whitehorn napsal(a): >>> >>> On 07/25/16 21:24, Warner Losh wrote: >>>> On Mon, Jul 25, 2016 at 10:48 AM, Nathan Whitehorn >>>> wrote: >>>>> On 07/25/16 09:32, Warner Losh wrote: >>>>>> On Mon, Jul 25, 2016 at 8:05 AM, Nathan Whitehorn >>>>>> wrote: >>>>>>> That wasn't my question. Are these particular device drivers >>>>>>> allocating >>>>>>> interrupts both on the GPIOs in their "interrupts" property (which >>>>>>> are >>>>>>> entirely GPIOs in this example) *and* on the GPIOs listed as >>>>>>> resources >>>>>>> but >>>>>>> not listed as interrupts? If they are, then you need a switching >>>>>>> mechanism, >>>>>>> but that seems pretty unlikely given the names of the >>>>>>> non-interrupt GPIOs >>>>>>> (they look like outputs). It would also be a somewhat deranged way >>>>>>> to set >>>>>>> up >>>>>>> a device tree -- not that that rules it out or anything. >>>>>> On Atmel, there's a situation that this covers, I think. >>>>>> >>>>>> The MCI device has an interrupt in the core: >>>>>> >>>>>> mmc0: mmc@fffa8000 { >>>>>> compatible = "atmel,hsmci"; >>>>>> reg = <0xfffa8000 0x600>; >>>>>> interrupts = <9 >>>>>> IRQ_TYPE_LEVEL_HIGH 0>; >>>>>> #address-cells = <1>; >>>>>> #size-cells = <0>; >>>>>> pinctrl-names = "default"; >>>>>> clocks = <&mci0_clk>; >>>>>> clock-names = "mci_clk"; >>>>>> status = "disabled"; >>>>>> }; >>>>>> >>>>>> and in other places wires in GPIO interrupts for things like card >>>>>> eject / insertion. >>>>>> >>>>>> mmc0: mmc@f0008000 { >>>>>> pinctrl-0 = < >>>>>> &pinctrl_board_mmc0 >>>>>> >>>>>> &pinctrl_mmc0_slot0_clk_cmd_dat0 >>>>>> >>>>>> &pinctrl_mmc0_slot0_dat1_3>; >>>>>> status = "okay"; >>>>>> slot@0 { >>>>>> reg = <0>; >>>>>> bus-width = <4>; >>>>>> cd-gpios = <&pioD 15 >>>>>> GPIO_ACTIVE_HIGH>; >>>>>> }; >>>>>> }; >>>>>> >>>>>> an interrupt is registered on the cd-gpios pin for when the card >>>>>> changes. >>>>>> At >>>>>> least in linux, FreeBSD doesn't (yet) implement this, but will >>>>>> someday if >>>>>> I get >>>>>> back to the armv6 atmel work I started (see at91-cosino.dts for >>>>>> example, >>>>>> there's >>>>>> others). >>>>>> >>>>>> I think this is an example of what you are asking about, or did I >>>>>> get >>>>>> lost in the >>>>>> twisty maze of conversation zigs and zags... >>>>>> >>>>>> Warner >>>>>> >>>>> Where we would run into (minor) problems is if the interrupt parent >>>>> for the >>>>> first mmc0 is the GPIO controller. More generally, if &pioD has >>>>> interrupt >>>>> children specified in some way that is not a >>>> high/whatever> >>>>> tuple somewhere else in the tree then you would have to have >>>>> methods to >>>>> parse both interrupt specifiers >>>>> as-obtained-from-interrupts-properties (or >>>>> equivalent) and specifiers as-obtained-from-gpio-properties. If the >>>>> tree >>>>> picks one format and sticks with it, you can get away with just the >>>>> one. >>>>> Glancing through the DTS source for this board, that doesn't appear >>>>> to be >>>>> the case and the property formatting is uniform, but I might have >>>>> missed >>>>> something in one of the many #includes. >>>> Interrupts and GPIO specifiers are different in subtle ways. The >>>> interrupt >>>> parent for mmc0 is an AIC, which is also the ultimate parent of the >>>> GPIO >>>> controller. >>> That is what it looked like. >>> >>>> But the properties for the GPIO pins that act as interrupts and >>>> the interrupt specifiers are different. >>> So there are devices with both interrupts = , >>> #interrupt-parent = <&gpio> and gpios = <&gpio bleh baz> where "Bleh >>> baz" is formatted different than "foo bar" and both are meant to be >>> treated as interrupts? >>> >>> It's fine if there are, but I haven't seen any such device trees yet. >> I think that yes, but I'm not ready to search all 1372 Linux DT files >> only for explicit example. >> But your question is invalid from beginning. >> 1) Format of each single 'interrupts' and '*-gpios' property is >> different because each single one are defined differently. >> 2) The question should not be 'Are there device' but 'Is this >> combination valid' >> >> Moreover, I'm curious why you want OFW property for gpio interrupts >> at all, >> given gpio can be instantiated by other entity (PCIe, USB). >> >> Please see: >> https://svnweb.freebsd.org/base/head/sys/dev/gpio/gpiobus.c?revision=301539&view=markup#l92 >> >> and, for example this one controller >> https://svnweb.freebsd.org/base/head/sys/arm/nvidia/tegra_gpio.c?revision=300149&view=markup#l575 >> > > Well, it's part of the device tree standard, so dealing with GPIO > properties seems reasonable where they occur. You could, of course, > have systems without device trees and GPIOs. I'm not sure where you > are going with this. > >> >>>>> As a general point, GPIO weirdness would be easy enough case to >>>>> handle if it >>>>> did come up (add some mapping method, as above) that I think we >>>>> shouldn't >>>>> worry too much about it from an architectural point of view. If a >>>>> board >>>>> appears that is set up this way, we can roll with the punches at >>>>> that point >>>>> and add whatever small amount of shim code that is required. It >>>>> would be >>>>> annoyance, sure, but not a real complication. >>>> I suspect that either I don't understand the issue, or we'll have >>>> such boards >>>> very quickly. The Atmel design is fairly clean in comparison to other >>>> franken-horrors >>>> I've seen... >>> People do weird things for sure. My point is just that the details of >>> how (implicit) GPIO interrupts are formatted just isn't that >>> important. It's easy enough to add special code for devices that are >>> set up in bizarre ways as they are spotted in the wild and that >>> special code is so minor that it doesn't matter for the design of the >>> API. This is a point I was just curious about, since I had never >>> actually seen device trees set up that way. >>> >>> The issue with this patch is, at its core, whether you have an >>> architecture that relies on the newbus hierarchy (like r301453) or >>> that allows links outside of that hierarchy that can cross branches or >>> go the "wrong" way, like the previously existing code. >> OK. The r301453 : >> - it removes unnecessary idea of virtual IRQs. I gave you some examples >> where virtual IRQ concept fails. > > I have never seen any of these examples. See [1]. > > The concept is *extremely* necessary, which is why both Linux and > FreeBSD decided to use it independently There is no way to handle > parent buses with a single rman and devices on multiple PICs with > overlapping interrupt ranges without them; neither is there a way to > decode arbitrary-length interrupt specifiers or to handle things like > MSIs. Please see the list of cases at > https://wiki.freebsd.org/Complicated_Interrupts and in my earlier > email for some examples of things that you just can't represent with > this new system, as far as I can tell. > >> Also, both variants needs attached PIC at bus_alloc_resource() time, >> so timing wasn't been changed. > > They absolutely do not, as I have explained repeatedly. Due to parent > devices with interrupts handled by their bus children, this is a hard > requirement of any workable system. This is not a theoretical issue; I > have lots of hardware like this. Nathan, I'm talking about pre /post r301453 state. Current INTRNG it has never supported. But yes, I'm fully understand why you want it. See [2]. > >> >> - it implements new BUS_MAP_INTR(). As I understand it, this is >> problematic for you, and I'm ready to change it. But I need more details >> than "it's fundamentally broken". > > Please explain (a) what cases it handles that the existing code and > does, and (b) how you would resolve each of the cases on the wiki page > I sent. > > The general issue is that it traces the newbus hierarchy, when > interrupts often do not, and so breaks when you have links to > as-yet-unattached parts of the hierarchy. It also relies on the > assigned "IRQ" numbers being nonoverlapping to avoid rman errors, but > that's a relatively minor thing. Well, from my point of view, only "get interrupt data from OFW/GPIO/.." part of bus_alloc_resource() follows bus hierarchy, real execution not changed. In any case, I see many possible variants how we can modify current implementation. For rest, see [2]. > >> >>> There are some other differences, of varying degrees of importance, >>> but that's the really fundamental one. I haven't seen any cases where >>> r301453 provides functionality absent in the already existing system, >>> but there seem to be a large number of cases (see the first email I >>> sent to freebsd-arch or >>> https://wiki.freebsd.org/Complicated_Interrupts) that the API in >>> r301453 cannot accommodate and that are needed to support a variety of >>> hardware. >> Which single feature has been removed by r301453? > > Nothing has been removed by it, because the normal code is intact. > However, the new code is less functional than the old code, so cannot > replace it. In particular, it is architecturally incapable of working > with the kinds of device trees found on PowerPC systems (see the wiki > page). That means we would have to keep both indefinitely, which is a > significant maintenance burden to no gain whatsoever. > -Nathan Firstly, please, ignore dependency problem. It will be explained later in [2]. [1] The original (your) INTRNG assume several things: - value of interrupt parent xref together with byte contents of 'interrupts' property forms some sort of 'key' (aka virtual IRQ) - byte contents of 'interrupts' property cannot be parsed in any way inside INTRNG core. - data forming this key are sufficient for subsequent mapping (and/or configuring) to real IRQ. - there must exist bidirectional unique relation between virtual and real IRQs. So one key (virtual IRQ) can be mappable to one and just one real IRQ. And only one virtual IRQ can be mapped to any given real IRQ. - we have cache that stores all observed 'keys' and associated real IRQ ) the they already mapped. Unfortunately, necessity of unique mapping is very problematic. We cannot handle situation, where shared interrupt is declared by different but compatible 'interrupts' properties. For example: ------------------------- foo1: bar@12345678 { interrupt-parent = <&pic1>; interrupts = <1 IRQ_TYPE_LEVEL_HIGH>; } foo2: bar@1234567C { interrupt-parent = <&pic1>; interrupts = <1 IRQ_TYPE_NONE>; } Is this valid (from DT point of view) configuration? I think that yes. Is this frequent configuration? I'm sure, isn't. Is this possible configuration? I'm afraid that yes. -------------------------- Next example are GPIO interrupts. These are encoded differently, so two 'keys' may point to to same real IRQ. After that we decided to change our approach and utilize standard resources and resource list entries to deliveryconfiguration data from various sources (OFW/GPIO/...) to consumers. [2] Each OFW device has a lot of dependencies. It must enable clocks, power regulators..., it must be taken from reset. All these action must be done before we can access single register. Most of these action can be done only with attached clock/power/reset controller. Within this, interrupts are not special. Moreover, 'cross type' circular dependencies are not that rare. I want to say: Resource dependencies are must solved (and resolved) at different level than is INTRNG. Blind resource allocation is not universal solution because given resource may/must be accessible immediately after allocation (in many cases). Unfortunately, and at time time, I known only one really working solution: staggered driver initialization combined with multipass bus initialization. I'm sorry, I'm not able to express all this accurately and clearly, but I've really tried... Michal From owner-freebsd-arch@freebsd.org Wed Jul 27 17:31:41 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 29B7FBA6F84; Wed, 27 Jul 2016 17:31:41 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from d.mail.sonic.net (d.mail.sonic.net [64.142.111.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id F2C5A16D0; Wed, 27 Jul 2016 17:31:40 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from zeppelin.tachypleus.net (75-101-50-44.static.sonic.net [75.101.50.44]) (authenticated bits=0) by d.mail.sonic.net (8.15.1/8.15.1) with ESMTPSA id u6RHJfQN032767 (version=TLSv1.2 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Wed, 27 Jul 2016 10:19:41 -0700 Subject: Re: INTRNG (Was: svn commit: r301453....) To: Michal Meloun References: <201606051620.u55GKD5S066398@repo.freebsd.org> <578E0B5D.3070105@FreeBSD.org> <578F6075.7010500@FreeBSD.org> <05a80ac6-4285-ec9d-36e9-2f92c609f746@freebsd.org> <57907B0F.9070204@FreeBSD.org> <9d2a224c-b787-2875-5984-a7a2354e8695@freebsd.org> <57934ABD.6010807@FreeBSD.org> <4e7a3e8f-cc21-f5f2-e3e0-4dbd554a4cd0@freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <57961549.4020105@FreeBSD.org> <57976867.6080705@FreeBSD.org> <5798E104.5020104@FreeBSD.org> Cc: "freebsd-arm@freebsd.org" , Svatopluk Kraus , "freebsd-arch@freebsd.org" From: Nathan Whitehorn Message-ID: Date: Wed, 27 Jul 2016 10:19:41 -0700 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:45.0) Gecko/20100101 Thunderbird/45.1.0 MIME-Version: 1.0 In-Reply-To: <5798E104.5020104@FreeBSD.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Sonic-CAuth: UmFuZG9tSVbQ0VGJh58dOgo9QFfuMJ7LNjMiw2aRGURT+ReQNg4twk6TiYUaVL4uTDA1IPPZ383KRl5eyuc0gDpaxPYFvlIMm6I0g5BUCYM= X-Sonic-ID: C;rKnFTR5U5hGPcpNwxPCmMQ== M;tjkDTh5U5hGPcpNwxPCmMQ== X-Spam-Flag: No X-Sonic-Spam-Details: 0.0/5.0 by cerberusd X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 27 Jul 2016 17:31:41 -0000 On 07/27/16 09:27, Michal Meloun wrote: [snip] >> The concept is *extremely* necessary, which is why both Linux and >> FreeBSD decided to use it independently There is no way to handle >> parent buses with a single rman and devices on multiple PICs with >> overlapping interrupt ranges without them; neither is there a way to >> decode arbitrary-length interrupt specifiers or to handle things like >> MSIs. Please see the list of cases at >> https://wiki.freebsd.org/Complicated_Interrupts and in my earlier >> email for some examples of things that you just can't represent with >> this new system, as far as I can tell. >> >>> Also, both variants needs attached PIC at bus_alloc_resource() time, >>> so timing wasn't been changed. >> They absolutely do not, as I have explained repeatedly. Due to parent >> devices with interrupts handled by their bus children, this is a hard >> requirement of any workable system. This is not a theoretical issue; I >> have lots of hardware like this. > Nathan, I'm talking about pre /post r301453 state. Current INTRNG it > has never supported. But yes, I'm fully understand why you want it. See > [2]. Current "INTRNG", at least on PowerPC, has supported this for 10 years. If it doesn't on ARM, it's because of some trivial implementation bug. > >>> - it implements new BUS_MAP_INTR(). As I understand it, this is >>> problematic for you, and I'm ready to change it. But I need more details >>> than "it's fundamentally broken". >> Please explain (a) what cases it handles that the existing code and >> does, and (b) how you would resolve each of the cases on the wiki page >> I sent. >> >> The general issue is that it traces the newbus hierarchy, when >> interrupts often do not, and so breaks when you have links to >> as-yet-unattached parts of the hierarchy. It also relies on the >> assigned "IRQ" numbers being nonoverlapping to avoid rman errors, but >> that's a relatively minor thing. > Well, from my point of view, only "get interrupt data from > OFW/GPIO/.." part of bus_alloc_resource() follows bus hierarchy, real > execution not changed. > In any case, I see many possible variants how we can modify current > implementation. For rest, see [2]. But that's an important part, for three reasons: 1. The PIC may not exist when bus_alloc_resource() is called 2. The bus parents have no useful information to contribute to this since the hierarchy doesn't go that way 3. If you try to use the interrupt pin as the resource ID, which r301453 does, you end up with rman conflicts if, for example, a bus has two children with interrupts on identically numbered pins on two different interrupt controllers. > >>> >>>> There are some other differences, of varying degrees of importance, >>>> but that's the really fundamental one. I haven't seen any cases where >>>> r301453 provides functionality absent in the already existing system, >>>> but there seem to be a large number of cases (see the first email I >>>> sent to freebsd-arch or >>>> https://wiki.freebsd.org/Complicated_Interrupts) that the API in >>>> r301453 cannot accommodate and that are needed to support a variety of >>>> hardware. >>> Which single feature has been removed by r301453? >> Nothing has been removed by it, because the normal code is intact. >> However, the new code is less functional than the old code, so cannot >> replace it. In particular, it is architecturally incapable of working >> with the kinds of device trees found on PowerPC systems (see the wiki >> page). That means we would have to keep both indefinitely, which is a >> significant maintenance burden to no gain whatsoever. >> -Nathan > Firstly, please, ignore dependency problem. It will be explained later > in [2]. Except that that isn't a workable solution (see the end) > > [1] > The original (your) INTRNG assume several things: > > - value of interrupt parent xref together with byte contents of > 'interrupts' property forms some sort of 'key' (aka virtual IRQ) > - byte contents of 'interrupts' property cannot be parsed in any way > inside INTRNG core. > - data forming this key are sufficient for subsequent mapping (and/or > configuring) to real IRQ. > - there must exist bidirectional unique relation between virtual and > real IRQs. So one key (virtual IRQ) can be mappable to one and just one > real IRQ. And only one virtual IRQ can be mapped to any given real > IRQ. > - we have cache that stores all observed 'keys' and associated real IRQ > ) the they already mapped. > > Unfortunately, necessity of unique mapping is very problematic. > We cannot handle situation, where shared interrupt is declared by > different but compatible 'interrupts' properties. There isn't actually a requirement of bidirectional uniqueness, as I explained the last time you brought this up. One "virtual" IRQ does need to map to exactly one interrupt specifier, but the reverse is not the case. The PIC driver is absolutely free to map and dispatch multiple virtual IRQs from the same shared interrupt pin, with no more overhead than you usually get from shared interrupt lines. > > For example: > ------------------------- > foo1: bar@12345678 { > interrupt-parent = <&pic1>; > interrupts = <1 IRQ_TYPE_LEVEL_HIGH>; > } > > foo2: bar@1234567C { > interrupt-parent = <&pic1>; > interrupts = <1 IRQ_TYPE_NONE>; > } > > Is this valid (from DT point of view) configuration? I think that yes. No, it's malformed and a violation of the standard. > Is this frequent configuration? I'm sure, isn't. > Is this possible configuration? I'm afraid that yes. People do in fact do deranged non-conformant things with device trees sometimes, unfortunately, so it is indeed worth worrying about. > -------------------------- > > Next example are GPIO interrupts. These are encoded differently, so two > 'keys' may point to to same real IRQ. > After that we decided to change our approach and utilize standard > resources and resource list entries to deliveryconfiguration data from > various sources (OFW/GPIO/...) to consumers. This is a non-issue. You make the GPIO controller a PIC, it handles interrupts however you like. If you got a weird device tree that uses two encodings (one for "interrupts" properties with GPIOs and one for "GPIO" properties on which you need to take interrupts but that weren't added to the "interrupts" list for mysterious reasons), you introduce a helper function for the GPIO case. Are there any actual problems with the pre-existing interrupt mapping code? I have not seen any so far. > [2] > Each OFW device has a lot of dependencies. It must enable clocks, power > regulators..., it must be taken from reset. All these action must be > done before we can access single register. > Most of these action can be done only with attached clock/power/reset > controller. > Within this, interrupts are not special. They are actually quite special in that the kernel enables them late and so you can defer setup. They are also special in that, for that reason, it is possible to design systems with circular dependency graphs with interrupts. It is not possible to do this with, for example, clocks: if I need to apply a clock to the clock controller with itself, the system is just not bootable and such a system will not be built or shipped. Interrupts need only happen later, after device setup, and so such systems *are* designed and shipped. The same is true for power. > Moreover, 'cross type' circular > dependencies are not that rare. > I want to say: > Resource dependencies are must solved (and resolved) at different level > than is INTRNG. > Blind resource allocation is not universal solution because given > resource may/must be accessible immediately after allocation (in many > cases). Absolutely: for GPIOs, clocks, power, etc. you want a system that looks different than the interrupt virtualization system. Probably with extra resource types and maybe with some API similar to r301453. But interrupts have different, and more complex, requirements and cannot be mapped this way. > > Unfortunately, and at time time, I known only one really working solution: > staggered driver initialization combined with multipass bus initialization. That does not solve the interrupt problem. If devices have interrupts on their own children, no amount of multipass initialization can possibly break the loop. Multipass would work for other kinds of resources (clocks, power, etc.) and is a perfect match for those resource types. A dynamic multipass (where a driver can return EAGAIN or something from attach if its resources don't exist yet) would work great. *But* in the specific, special case of interrupts, it is not a workable model. You could imagine some change to initialization that gives devices a late attach and an early attach method and does interrupts in the late part, but that is hugely invasive change that would need to touch every single driver in the tree -- to solve a problem that is already solved by interrupt virtualization. -Nathan > I'm sorry, I'm not able to express all this accurately and clearly, but > I've really tried... > Michal > > > _______________________________________________ > freebsd-arch@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/freebsd-arch > To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" > From owner-freebsd-arch@freebsd.org Wed Jul 27 19:38:13 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 1BD36BA64D3; Wed, 27 Jul 2016 19:38:13 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: from mail-io0-x22b.google.com (mail-io0-x22b.google.com [IPv6:2607:f8b0:4001:c06::22b]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id D78911E5D; Wed, 27 Jul 2016 19:38:12 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: by mail-io0-x22b.google.com with SMTP id b62so79823326iod.3; Wed, 27 Jul 2016 12:38:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=63dXpzYpNTjTyhQYIGqSPAsI245KkU/gHEQP+yB1GQA=; b=rLebOTx3ZJTpySs/oPxZ24u4RUsT5E6R1uT/GKFeQ86Ul+fOjH7aoSBWrmuEejKLKv SB+ZG5VZDmkveMGgUgqq+jscnm976oHRH1kfLEjmd0idfLPQGUmMhexSKUdSziFmnyoj 5PdxwkBKuJrxjiXjTx8/jOfI9z2uoAIqYFeTZ85cTmYd4eMPBItNzVooAUe3UhhKWSkn 5n+8pId49qtM0sWfsQ8UT84MDzFYdHXB9gQqRVSGL/EzNj1QYvjfuJHNp0jqrvqWtd4+ XzBVE0uSr08HdGu4Jhf52TghldjBd09uymuCnXX3yFGp49LUTUSyLYJIKI9alrZ0Ncf5 9eyQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=63dXpzYpNTjTyhQYIGqSPAsI245KkU/gHEQP+yB1GQA=; b=aPuGtII+DfrMeFDge887yxt5Tqy6I2OGYV660AddVdRw+2+qPWCwm4e1KhCy0O4V3t sUz5CxAsVOhSrhg9CbkcUx9UZmz3sYZyfymw+3EIh9je8rvA02uWZNY4AaNnnRAW7AEC /gEd7H7xJ2PQPNNp54L7iJOARJA3iZg18ejmbvxYBZ/dFz9RFmkVlUPpemdWx/M+WjEo FBcN2shXX9UjzumZhSA+UEj920oYC3ejZ+pP9plII/COFaf1XR0n1B6PK+wgX8gGUn+m yhY/sCoKEEfzo3k9PHf0DNxzJUf03GNG0R6ht/jEnVYbgKPCtDbAe09eKz/rEFXM0GuH oqrg== X-Gm-Message-State: AEkoouvuduXTmAPdgcCAKApwGQB48qERBuj/yP+sb9dEEKSVIzkVjBzYw60ufY1sGGQ4Ckuit50PSWSmmTlLrA== X-Received: by 10.107.144.10 with SMTP id s10mr32964662iod.165.1469648292079; Wed, 27 Jul 2016 12:38:12 -0700 (PDT) MIME-Version: 1.0 Received: by 10.36.141.129 with HTTP; Wed, 27 Jul 2016 12:38:11 -0700 (PDT) In-Reply-To: References: <201606051620.u55GKD5S066398@repo.freebsd.org> <578E0B5D.3070105@FreeBSD.org> <578F6075.7010500@FreeBSD.org> <05a80ac6-4285-ec9d-36e9-2f92c609f746@freebsd.org> <57907B0F.9070204@FreeBSD.org> <9d2a224c-b787-2875-5984-a7a2354e8695@freebsd.org> <57934ABD.6010807@FreeBSD.org> <4e7a3e8f-cc21-f5f2-e3e0-4dbd554a4cd0@freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <57961549.4020105@FreeBSD.org> <57976867.6080705@FreeBSD.org> <5798E104.5020104@FreeBSD.org> From: Adrian Chadd Date: Wed, 27 Jul 2016 12:38:11 -0700 Message-ID: Subject: Re: INTRNG (Was: svn commit: r301453....) To: Nathan Whitehorn Cc: Michal Meloun , "freebsd-arm@freebsd.org" , Svatopluk Kraus , "freebsd-arch@freebsd.org" Content-Type: text/plain; charset=UTF-8 X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 27 Jul 2016 19:38:13 -0000 [snip] can the ARM specific bits be added to Nathan's wiki page describing interrupt stuff? I'd like to see some of this complication documented so people see the full variety of hilarity. Thanks! -adrian From owner-freebsd-arch@freebsd.org Wed Jul 27 20:44:58 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 6324BBA66F5 for ; Wed, 27 Jul 2016 20:44:58 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: from mail-io0-x236.google.com (mail-io0-x236.google.com [IPv6:2607:f8b0:4001:c06::236]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 287EF1CF1 for ; Wed, 27 Jul 2016 20:44:58 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: by mail-io0-x236.google.com with SMTP id b62so81409372iod.3 for ; Wed, 27 Jul 2016 13:44:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:from:date:message-id:subject:to; bh=GzYRufWklB9y0W3pnxvAfLz5QPJ9k2J2bXSR/Q+MyqQ=; b=dYlc4lWxx7xwPneD4oyXGK5uBSA0y/5QMKDboSzj9FPoue0KmlFN9ec9vpE6BG0JTa AifA+HtL5ZpYHRAGAbQJKeZ+LfRT01nnTEqftOLoz/z2KiS4439v2hWVTDZCF48Od5d0 wNP2m525VL9cTT+2ePjdsHrl/ITeMC2k6r113xFSwkBrzRBGIpD12Ed3hljLxXqf0Blv nq6dr7RNrgepzAIT5u4mDvVYW2HllNfJKSal25GMbUBKO0w/5QUx+SDOPPo3v7EU9UDW z+NIX8vC22PlyPIhAPfVrQgcpBl8/sn+oDmZu2lB1N22P5K+63Jk8ma8WxJDHY27VeoO KvAQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:sender:from:date:message-id:subject :to; bh=GzYRufWklB9y0W3pnxvAfLz5QPJ9k2J2bXSR/Q+MyqQ=; b=WaYy+i2JCkm0bUS8iDI5gIuGni12i1yjFRE79f5D4h8VAANFIO9pBfSpgNxSkcA8tS 15vXpc9d7t3uWi44YxRlBO7+KKrPJAXeXRKzYxBqCAJIFdRc33MMC42LzHLWQe1mjSXC z5iiCrUlXazv3Dpov0NWQqWWHr3kYD9Uka2oFXt0FeS53aXT50dXkbKPQI8mOmG3Mp/Q MGK8ZktVxjanIsUeh3bkC6Ew8swMgFGMe96ax2AouFD6dpT/FuuTM/uOuHsd9Yxx3lCh pXsFvORt7nplOId3vXAff1/swecia2sptzYIE4sSpbLw2yixzKeuB8/9Bqx6cexEyYIU PWCg== X-Gm-Message-State: AEkooutUqAK6aQsHxL77sH4NoQMxa8BwT4OE/iAMVmXeWYj2HI6+c65smwac2s5JDCzAK8DR2OS01+yxdfoLLA== X-Received: by 10.107.144.10 with SMTP id s10mr33224399iod.165.1469652297315; Wed, 27 Jul 2016 13:44:57 -0700 (PDT) MIME-Version: 1.0 Sender: adrian.chadd@gmail.com Received: by 10.36.141.129 with HTTP; Wed, 27 Jul 2016 13:44:56 -0700 (PDT) From: Adrian Chadd Date: Wed, 27 Jul 2016 13:44:56 -0700 X-Google-Sender-Auth: eIC7G3i4_ej6LNndfAvY4YcSZkI Message-ID: Subject: indent fixes To: "freebsd-arch@freebsd.org" Content-Type: text/plain; charset=UTF-8 X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 27 Jul 2016 20:44:58 -0000 hi, There's a review with fixes to our indent: https://reviews.freebsd.org/D6966 Would anyone mind reviewing it? He'd like some more feedback. Thanks! -a From owner-freebsd-arch@freebsd.org Wed Jul 27 21:00:35 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 0E82CBA6C75 for ; Wed, 27 Jul 2016 21:00:35 +0000 (UTC) (envelope-from shawn.webb@hardenedbsd.org) Received: from mail-qk0-x230.google.com (mail-qk0-x230.google.com [IPv6:2607:f8b0:400d:c09::230]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id B96AB1DEC for ; Wed, 27 Jul 2016 21:00:34 +0000 (UTC) (envelope-from shawn.webb@hardenedbsd.org) Received: by mail-qk0-x230.google.com with SMTP id x1so45846336qkb.3 for ; Wed, 27 Jul 2016 14:00:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=hardenedbsd-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=UN0/vm1WEuLnpYK0i2miRok7cPDuD5THYVev9oE5lWk=; b=KqqFcIJTjTE8IFm9gJN8oCFlcHjNvigISsvtqAcM3w7oTQv9O9R2OS6lRi5IhQHKoH 2cOgXaW3CYXnit7JkoR0nbVebzEylsjkBDVftj+Ez0n/YdItEVfUuTpws87VcP4+8YIK 6pXuZ7iK2J+cWeuVwPvTxkysEp+It6HGVKvdEJuDShen9S6aiI+N2Xi2bBPcRs9Np86n elJtmiD6HlVQphBxNCAGR1paT8SGoCPITY0wqoIYAxbo/8kmF0CB/Z4ncdIBGikQVWno 66HBG+Jxy/+y24nFOylxYhKnHJmytay08ZsbIaDC/Bv1b4LT8D0pMEw3YroukJNVyRAr esjQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=UN0/vm1WEuLnpYK0i2miRok7cPDuD5THYVev9oE5lWk=; b=ApIFQmO0c8BXaNvdElldm/3m5EwoRBJA+in29e9jqMDGDQbmge96AFf3zPf3m92bfq ee+60dKEpJuW+hxBuwesYN5bP7n2Ty/2A1OG8Xr4Dgo7KWQRt6EecJ8kWsgMdKCKKZGU Apb1cKdQLwhWov6jsnScypqAJ9Une+P5ZmH3bD1nNuySaXgXTNEvVh5tEcgOzHuVU07o y2GqjQEIY/rzaZ6SBKFINrOfAmxBpYXVWUCz+P0xoPB7NJIN4JakNeRhg/BVOZRYpxqC si6jWXPAORMTtDZyZevq6xbsiOIZGql79cBn6o7sGZ9k4m3FQnUAi9zbPnJL91K2i7ix lFhQ== X-Gm-Message-State: AEkoouuwShJlpUwXsmhIFrT6ZjNMFdSQzN1makhTfwRtuhSaftInWNgsuNGkAeM+fa83cmNB X-Received: by 10.55.147.70 with SMTP id v67mr37557069qkd.32.1469653233895; Wed, 27 Jul 2016 14:00:33 -0700 (PDT) Received: from mutt-hardenedbsd (pool-100-16-217-171.bltmmd.fios.verizon.net. [100.16.217.171]) by smtp.gmail.com with ESMTPSA id n6sm5327854qke.31.2016.07.27.14.00.32 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 27 Jul 2016 14:00:32 -0700 (PDT) Date: Wed, 27 Jul 2016 17:00:30 -0400 From: Shawn Webb To: Adrian Chadd Cc: "freebsd-arch@freebsd.org" Subject: Re: indent fixes Message-ID: <20160727210030.GE13428@mutt-hardenedbsd> References: MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="gDGSpKKIBgtShtf+" Content-Disposition: inline In-Reply-To: X-Operating-System: FreeBSD mutt-hardenedbsd 12.0-CURRENT-HBSD FreeBSD 12.0-CURRENT-HBSD X-PGP-Key: http://pgp.mit.edu/pks/lookup?op=vindex&search=0x6A84658F52456EEE User-Agent: Mutt/1.6.1 (2016-04-27) X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 27 Jul 2016 21:00:35 -0000 --gDGSpKKIBgtShtf+ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jul 27, 2016 at 01:44:56PM -0700, Adrian Chadd wrote: > hi, >=20 > There's a review with fixes to our indent: >=20 > https://reviews.freebsd.org/D6966 >=20 > Would anyone mind reviewing it? He'd like some more feedback. I just added a few comments after glancing at it (not a full review). Thanks, --=20 Shawn Webb Cofounder and Security Engineer HardenedBSD GPG Key ID: 0x6A84658F52456EEE GPG Key Fingerprint: 2ABA B6BD EF6A F486 BE89 3D9E 6A84 658F 5245 6EEE --gDGSpKKIBgtShtf+ Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJXmSDrAAoJEGqEZY9SRW7ulIIQAI80DjIhTFVjhkUylKSWJ2MJ xnkE+MVMNsSptH8fUikNLmYHJEsareEU+5h4652fS4hZqAGqEux7BLLipBAqJf0e Iv6Ux0C0Z0Hv2P5xsDjLfg+qMHR7534ypjUONcxb13XuntFQYhKvjcOpy9Ui1x/u w2RzaSFtHZMoc30KudxgHwnG3DJcBBROQr2qOTU2iSh+APoor8jm11cYphVMqxc2 n69u86JjspPJR5e029hLKgV/H5jo1Y3L6OxATt772u8JN7yCz3ybqihfsuZfb/X/ ywoIUCcnKHfAJ6stmXD87c48qYrUZM5hF/+o2GUqC7mRqFvi3d2LgpvhPXUl9U5q qHCwYtfGdhyYqNVdv3R0i89FG1eud9XotmTweu9k2tim4i7zkXHj/wMfM8SNd2uc NPcWgjuxHA9zrz/gORg6lH4UcK4RgoBANP26kgE5+az+4wxTkTblAM7b1PIOcPZR NnDjNqfUYVQLlo6ENeVRWREcMEDvj8HUN5+l3UZNYH8bFQNwPyyLH0/6yXHxCZI7 PuQY3e8zbX3qn0Nd/4ny0YHbtHNg7cECKEkJI2u5gOMJfa+kAUn/8tytiz8jVd6j bDawZjOJ8GQQ9yKXzMJgSBLOFvsC9GAvYTZq0/VZQb6MVWgi/TrAM7W/xl5AoEKE B/1rY/O5/LWNkof6BPbn =D0/q -----END PGP SIGNATURE----- --gDGSpKKIBgtShtf+-- From owner-freebsd-arch@freebsd.org Thu Jul 28 15:33:19 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 5BB12BA670E; Thu, 28 Jul 2016 15:33:19 +0000 (UTC) (envelope-from mmel@FreeBSD.org) Received: from mail.miracle.cz (mail.miracle.cz [193.84.128.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mail.miracle.cz", Issuer "Miracle Group Root CA" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 15D5C1A84; Thu, 28 Jul 2016 15:33:18 +0000 (UTC) (envelope-from mmel@FreeBSD.org) Received: from [193.84.128.50] (meloun.ad.miracle.cz [193.84.128.50]) by mail.miracle.cz (Postfix) with ESMTPSA id 7F2003ACCF; Thu, 28 Jul 2016 17:33:15 +0200 (CEST) Subject: Re: INTRNG (Was: svn commit: r301453....) To: Nathan Whitehorn References: <201606051620.u55GKD5S066398@repo.freebsd.org> <578F6075.7010500@FreeBSD.org> <05a80ac6-4285-ec9d-36e9-2f92c609f746@freebsd.org> <57907B0F.9070204@FreeBSD.org> <9d2a224c-b787-2875-5984-a7a2354e8695@freebsd.org> <57934ABD.6010807@FreeBSD.org> <4e7a3e8f-cc21-f5f2-e3e0-4dbd554a4cd0@freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <57961549.4020105@FreeBSD.org> <57976867.6080705@FreeBSD.org> <5798E104.5020104@FreeBSD.org> Cc: "freebsd-arm@freebsd.org" , Svatopluk Kraus , "freebsd-arch@freebsd.org" From: Michal Meloun X-Enigmail-Draft-Status: N1110 Message-ID: <579A25BB.8070206@FreeBSD.org> Date: Thu, 28 Jul 2016 17:33:15 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.4.3 (mail.miracle.cz); Thu, 28 Jul 2016 17:33:15 +0200 (CEST) X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Jul 2016 15:33:19 -0000 Dne 27.07.2016 v 19:19 Nathan Whitehorn napsal(a): > > > On 07/27/16 09:27, Michal Meloun wrote: > > > [snip] >>> The concept is *extremely* necessary, which is why both Linux and >>> FreeBSD decided to use it independently There is no way to handle >>> parent buses with a single rman and devices on multiple PICs with >>> overlapping interrupt ranges without them; neither is there a way to >>> decode arbitrary-length interrupt specifiers or to handle things like >>> MSIs. Please see the list of cases at >>> https://wiki.freebsd.org/Complicated_Interrupts and in my earlier >>> email for some examples of things that you just can't represent with >>> this new system, as far as I can tell. >>> >>>> Also, both variants needs attached PIC at bus_alloc_resource() >>>> time, >>>> so timing wasn't been changed. >>> They absolutely do not, as I have explained repeatedly. Due to parent >>> devices with interrupts handled by their bus children, this is a hard >>> requirement of any workable system. This is not a theoretical issue; I >>> have lots of hardware like this. >> Nathan, I'm talking about pre /post r301453 state. Current INTRNG it >> has never supported. But yes, I'm fully understand why you want it. See >> [2]. > > Current "INTRNG", at least on PowerPC, has supported this for 10 > years. If it doesn't on ARM, it's because of some trivial > implementation bug. >> >>>> - it implements new BUS_MAP_INTR(). As I understand it, this is >>>> problematic for you, and I'm ready to change it. But I need more >>>> details >>>> than "it's fundamentally broken". >>> Please explain (a) what cases it handles that the existing code and >>> does, and (b) how you would resolve each of the cases on the wiki page >>> I sent. >>> >>> The general issue is that it traces the newbus hierarchy, when >>> interrupts often do not, and so breaks when you have links to >>> as-yet-unattached parts of the hierarchy. It also relies on the >>> assigned "IRQ" numbers being nonoverlapping to avoid rman errors, but >>> that's a relatively minor thing. >> Well, from my point of view, only "get interrupt data from >> OFW/GPIO/.." part of bus_alloc_resource() follows bus hierarchy, real >> execution not changed. >> In any case, I see many possible variants how we can modify current >> implementation. For rest, see [2]. > > But that's an important part, for three reasons: > 1. The PIC may not exist when bus_alloc_resource() is called > 2. The bus parents have no useful information to contribute to this > since the hierarchy doesn't go that way See [2] > 3. If you try to use the interrupt pin as the resource ID, which > r301453 does, you end up with rman conflicts if, for example, a bus > has two children with interrupts on identically numbered pins on two > different interrupt controllers. No, r301453 doesn't do this. Index to global interrupt source table (irq_sources) is used as resource ID (and r301453 not changed it). > >> >>>> >>>>> There are some other differences, of varying degrees of importance, >>>>> but that's the really fundamental one. I haven't seen any cases where >>>>> r301453 provides functionality absent in the already existing system, >>>>> but there seem to be a large number of cases (see the first email I >>>>> sent to freebsd-arch or >>>>> https://wiki.freebsd.org/Complicated_Interrupts) that the API in >>>>> r301453 cannot accommodate and that are needed to support a >>>>> variety of >>>>> hardware. >>>> Which single feature has been removed by r301453? >>> Nothing has been removed by it, because the normal code is intact. >>> However, the new code is less functional than the old code, so cannot >>> replace it. In particular, it is architecturally incapable of working >>> with the kinds of device trees found on PowerPC systems (see the wiki >>> page). That means we would have to keep both indefinitely, which is a >>> significant maintenance burden to no gain whatsoever. >>> -Nathan >> Firstly, please, ignore dependency problem. It will be explained later >> in [2]. > > Except that that isn't a workable solution (see the end) > >> >> [1] >> The original (your) INTRNG assume several things: >> >> - value of interrupt parent xref together with byte contents of >> 'interrupts' property forms some sort of 'key' (aka virtual IRQ) >> - byte contents of 'interrupts' property cannot be parsed in any way >> inside INTRNG core. >> - data forming this key are sufficient for subsequent mapping (and/or >> configuring) to real IRQ. >> - there must exist bidirectional unique relation between virtual and >> real IRQs. So one key (virtual IRQ) can be mappable to one and just >> one >> real IRQ. And only one virtual IRQ can be mapped to any given real >> IRQ. >> - we have cache that stores all observed 'keys' and associated real IRQ >> ) the they already mapped. >> >> Unfortunately, necessity of unique mapping is very problematic. >> We cannot handle situation, where shared interrupt is declared by >> different but compatible 'interrupts' properties. > > There isn't actually a requirement of bidirectional uniqueness, as I > explained the last time you brought this up. One "virtual" IRQ does > need to map to exactly one interrupt specifier, but the reverse is not > the case. The PIC driver is absolutely free to map and dispatch > multiple virtual IRQs from the same shared interrupt pin, with no more > overhead than you usually get from shared interrupt lines. Oki, lets me to expand this a little bit more. The pre r301453 INTRNG[ARM] does this: - ofw_bus_intr_to_rl() reads and pre-parses 'interrupts' property into 'key'. - cache is searched for duplicates and new entry is created. - position of this entry in cache is used as resource ID. - given resource ID is stored to RL and is later used in bus_alloc_resource() I'm right ? In the above model, individual cache entries may be shared are referenced by multiple entities (PIC, RL, ..). Due to this, life cycle of cache entries is not well defined and we probably must use some sort of reference counting to control it. But, as you wrote, INTRNG code is "absolutely free to map and dispatch multiple virtual IRQs from the same shared interrupt pin" (allow me to expand this also to INTRNG code itself) . In attempt to simply situation abound cache, in first step, we removed duplicates check from above model. This also removes unpleasantly sharing of entries. This step converts cache to some sort of duplicate of resource list entries. Each existing RLE have exactly one corresponding entry in cache. Also, the cache is not needed for INTRNG core. At this point, its only transfers data from ofw_bus_intr_to_rl() to consumers PIC. So, if we are able to attach 'key' to RLE, then we can remove the whole cache machinery from INTRNG. Do you agree? > >> >> For example: >> ------------------------- >> foo1: bar@12345678 { >> interrupt-parent = <&pic1>; >> interrupts = <1 IRQ_TYPE_LEVEL_HIGH>; >> } >> >> foo2: bar@1234567C { >> interrupt-parent = <&pic1>; >> interrupts = <1 IRQ_TYPE_NONE>; >> } >> >> Is this valid (from DT point of view) configuration? I think that yes. > > No, it's malformed and a violation of the standard. Can you give me pointer, please? I'm not able to find anything about this... > >> Is this frequent configuration? I'm sure, isn't. >> Is this possible configuration? I'm afraid that yes. > > People do in fact do deranged non-conformant things with device trees > sometimes, unfortunately, so it is indeed worth worrying about. > >> -------------------------- >> >> Next example are GPIO interrupts. These are encoded differently, so two >> 'keys' may point to to same real IRQ. >> After that we decided to change our approach and utilize standard >> resources and resource list entries to deliveryconfiguration data from >> various sources (OFW/GPIO/...) to consumers. > > This is a non-issue. You make the GPIO controller a PIC, it handles > interrupts however you like. If you got a weird device tree that uses > two encodings (one for "interrupts" properties with GPIOs and one for > "GPIO" properties on which you need to take interrupts but that > weren't added to the "interrupts" list for mysterious reasons), you > introduce a helper function for the GPIO case. > > Are there any actual problems with the pre-existing interrupt mapping > code? I have not seen any so far. > >> [2] >> Each OFW device has a lot of dependencies. It must enable clocks, power >> regulators..., it must be taken from reset. All these action must be >> done before we can access single register. >> Most of these action can be done only with attached clock/power/reset >> controller. >> Within this, interrupts are not special. > > They are actually quite special in that the kernel enables them late > and so you can defer setup. They are also special in that, for that > reason, it is possible to design systems with circular dependency > graphs with interrupts. It is not possible to do this with, for > example, clocks: if I need to apply a clock to the clock controller > with itself, the system is just not bootable and such a system will > not be built or shipped. Interrupts need only happen later, after > device setup, and so such systems *are* designed and shipped. The same > is true for power. The G5 problem is standard 'cross type' circular dependency. You must (at BUS_PASS_BUS) initialize memory address window of PCI bridge and start bus enumeration. Rest of PCI bridge initialization (including all IRQ stuff) must be postponed to any later pass. Of course, all interrupt controllers must be attached at BUS_PASS_INTERRUPT. Or I missed something? >> Moreover, 'cross type' circular >> dependencies are not that rare. >> I want to say: >> Resource dependencies are must solved (and resolved) at different level >> than is INTRNG. >> Blind resource allocation is not universal solution because given >> resource may/must be accessible immediately after allocation (in many >> cases). > > Absolutely: for GPIOs, clocks, power, etc. you want a system that > looks different than the interrupt virtualization system. Probably > with extra resource types and maybe with some API similar to r301453. > But interrupts have different, and more complex, requirements and > cannot be mapped this way. > >> >> Unfortunately, and at time time, I known only one really working >> solution: >> staggered driver initialization combined with multipass bus >> initialization. > > That does not solve the interrupt problem. If devices have interrupts > on their own children, no amount of multipass initialization can > possibly break the loop. Multipass would work for other kinds of > resources (clocks, power, etc.) and is a perfect match for those > resource types. A dynamic multipass (where a driver can return EAGAIN > or something from attach if its resources don't exist yet) would work > great. > > *But* in the specific, special case of interrupts, it is not a > workable model. You could imagine some change to initialization that > gives devices a late attach and an early attach method and does > interrupts in the late part, but that is hugely invasive change that > would need to touch every single driver in the tree -- to solve a > problem that is already solved by interrupt virtualization. > -Nathan > >> I'm sorry, I'm not able to express all this accurately and clearly, but >> I've really tried... >> Michal >> >> >> _______________________________________________ >> freebsd-arch@freebsd.org mailing list >> https://lists.freebsd.org/mailman/listinfo/freebsd-arch >> To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" >> > From owner-freebsd-arch@freebsd.org Thu Jul 28 17:34:29 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 9526CBA7C46; Thu, 28 Jul 2016 17:34:29 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from c.mail.sonic.net (c.mail.sonic.net [64.142.111.80]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 7817C1876; Thu, 28 Jul 2016 17:34:28 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from aurora.physics.berkeley.edu (aurora.physics.berkeley.edu [128.32.117.67]) (authenticated bits=0) by c.mail.sonic.net (8.15.1/8.15.1) with ESMTPSA id u6SHNUjT014395 (version=TLSv1.2 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Thu, 28 Jul 2016 10:23:31 -0700 Subject: Re: INTRNG (Was: svn commit: r301453....) To: Michal Meloun References: <201606051620.u55GKD5S066398@repo.freebsd.org> <578F6075.7010500@FreeBSD.org> <05a80ac6-4285-ec9d-36e9-2f92c609f746@freebsd.org> <57907B0F.9070204@FreeBSD.org> <9d2a224c-b787-2875-5984-a7a2354e8695@freebsd.org> <57934ABD.6010807@FreeBSD.org> <4e7a3e8f-cc21-f5f2-e3e0-4dbd554a4cd0@freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <57961549.4020105@FreeBSD.org> <57976867.6080705@FreeBSD.org> <5798E104.5020104@FreeBSD.org> <579A25BB.8070206@FreeBSD.org> Cc: "freebsd-arm@freebsd.org" , Svatopluk Kraus , "freebsd-arch@freebsd.org" From: Nathan Whitehorn Message-ID: <30790e40-58b4-3371-c0f0-b7545571f389@freebsd.org> Date: Thu, 28 Jul 2016 10:23:30 -0700 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:45.0) Gecko/20100101 Thunderbird/45.1.0 MIME-Version: 1.0 In-Reply-To: <579A25BB.8070206@FreeBSD.org> X-Sonic-CAuth: UmFuZG9tSVbdmXzutuUe3AvM/XnDGFJxM3mv5WJ7H+35zkiJ+klXdGk/muOvp0UKqptpST0bVl5UYaMmEBlQ8zJ6zTA+cBGsQty6DFa0pM4= X-Sonic-ID: C;dnj/AOhU5hGYbJtMTlz00w== M;IAFKAehU5hGYbJtMTlz00w== X-Spam-Flag: No X-Sonic-Spam-Details: 0.0/5.0 by cerberusd Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.22 X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Jul 2016 17:34:29 -0000 On 07/28/16 08:33, Michal Meloun wrote: > Dne 27.07.2016 v 19:19 Nathan Whitehorn napsal(a): >> >> On 07/27/16 09:27, Michal Meloun wrote: >> >> >> [snip] >>>> The concept is *extremely* necessary, which is why both Linux and >>>> FreeBSD decided to use it independently There is no way to handle >>>> parent buses with a single rman and devices on multiple PICs with >>>> overlapping interrupt ranges without them; neither is there a way to >>>> decode arbitrary-length interrupt specifiers or to handle things like >>>> MSIs. Please see the list of cases at >>>> https://wiki.freebsd.org/Complicated_Interrupts and in my earlier >>>> email for some examples of things that you just can't represent with >>>> this new system, as far as I can tell. >>>> >>>>> Also, both variants needs attached PIC at bus_alloc_resource() >>>>> time, >>>>> so timing wasn't been changed. >>>> They absolutely do not, as I have explained repeatedly. Due to parent >>>> devices with interrupts handled by their bus children, this is a hard >>>> requirement of any workable system. This is not a theoretical issue; I >>>> have lots of hardware like this. >>> Nathan, I'm talking about pre /post r301453 state. Current INTRNG it >>> has never supported. But yes, I'm fully understand why you want it. See >>> [2]. >> Current "INTRNG", at least on PowerPC, has supported this for 10 >> years. If it doesn't on ARM, it's because of some trivial >> implementation bug. >>>>> - it implements new BUS_MAP_INTR(). As I understand it, this is >>>>> problematic for you, and I'm ready to change it. But I need more >>>>> details >>>>> than "it's fundamentally broken". >>>> Please explain (a) what cases it handles that the existing code and >>>> does, and (b) how you would resolve each of the cases on the wiki page >>>> I sent. >>>> >>>> The general issue is that it traces the newbus hierarchy, when >>>> interrupts often do not, and so breaks when you have links to >>>> as-yet-unattached parts of the hierarchy. It also relies on the >>>> assigned "IRQ" numbers being nonoverlapping to avoid rman errors, but >>>> that's a relatively minor thing. >>> Well, from my point of view, only "get interrupt data from >>> OFW/GPIO/.." part of bus_alloc_resource() follows bus hierarchy, real >>> execution not changed. >>> In any case, I see many possible variants how we can modify current >>> implementation. For rest, see [2]. >> But that's an important part, for three reasons: >> 1. The PIC may not exist when bus_alloc_resource() is called >> 2. The bus parents have no useful information to contribute to this >> since the hierarchy doesn't go that way > See [2] > >> 3. If you try to use the interrupt pin as the resource ID, which >> r301453 does, you end up with rman conflicts if, for example, a bus >> has two children with interrupts on identically numbered pins on two >> different interrupt controllers. > No, r301453 doesn't do this. > Index to global interrupt source table (irq_sources) is used as resource > ID (and r301453 not changed it). Ah, I see that now. Apologies for the confusion. So, to understand fully: your major complaint about the existing code is that it uses arbitrary numbers reflecting some table lookup value as IRQ numbers, which r301453 *also* does? >>>>> >>>>>> There are some other differences, of varying degrees of importance, >>>>>> but that's the really fundamental one. I haven't seen any cases where >>>>>> r301453 provides functionality absent in the already existing system, >>>>>> but there seem to be a large number of cases (see the first email I >>>>>> sent to freebsd-arch or >>>>>> https://wiki.freebsd.org/Complicated_Interrupts) that the API in >>>>>> r301453 cannot accommodate and that are needed to support a >>>>>> variety of >>>>>> hardware. >>>>> Which single feature has been removed by r301453? >>>> Nothing has been removed by it, because the normal code is intact. >>>> However, the new code is less functional than the old code, so cannot >>>> replace it. In particular, it is architecturally incapable of working >>>> with the kinds of device trees found on PowerPC systems (see the wiki >>>> page). That means we would have to keep both indefinitely, which is a >>>> significant maintenance burden to no gain whatsoever. >>>> -Nathan >>> Firstly, please, ignore dependency problem. It will be explained later >>> in [2]. >> Except that that isn't a workable solution (see the end) >> >>> [1] >>> The original (your) INTRNG assume several things: >>> >>> - value of interrupt parent xref together with byte contents of >>> 'interrupts' property forms some sort of 'key' (aka virtual IRQ) >>> - byte contents of 'interrupts' property cannot be parsed in any way >>> inside INTRNG core. >>> - data forming this key are sufficient for subsequent mapping (and/or >>> configuring) to real IRQ. >>> - there must exist bidirectional unique relation between virtual and >>> real IRQs. So one key (virtual IRQ) can be mappable to one and just >>> one >>> real IRQ. And only one virtual IRQ can be mapped to any given real >>> IRQ. >>> - we have cache that stores all observed 'keys' and associated real IRQ >>> ) the they already mapped. >>> >>> Unfortunately, necessity of unique mapping is very problematic. >>> We cannot handle situation, where shared interrupt is declared by >>> different but compatible 'interrupts' properties. >> There isn't actually a requirement of bidirectional uniqueness, as I >> explained the last time you brought this up. One "virtual" IRQ does >> need to map to exactly one interrupt specifier, but the reverse is not >> the case. The PIC driver is absolutely free to map and dispatch >> multiple virtual IRQs from the same shared interrupt pin, with no more >> overhead than you usually get from shared interrupt lines. > Oki, lets me to expand this a little bit more. > > The pre r301453 INTRNG[ARM] does this: > - ofw_bus_intr_to_rl() reads and pre-parses 'interrupts' property > into 'key'. Sometimes. For some buses. ofw_bus_intr_to_rl() is a function of very limited application that should really only be used by simplebus; how that works depends on the bus bindings. There are many cases (e.g. MSIs, PCI buses generally) where interrupts are set up by a different route. > - cache is searched for duplicates and new entry is created. > - position of this entry in cache is used as resource ID. Resource ID is arbitrary, but it could be this. > - given resource ID is stored to RL and is later used in > bus_alloc_resource() Yes. > I'm right ? > > In the above model, individual cache entries may be shared are > referenced by multiple entities (PIC, RL, ..). Due to this, life cycle > of cache entries is not well defined and we probably must use some sort > of reference counting to control it. Why? Interrupts are not unmapped and remapped with unique values often enough (or, really, at all) to warrant even thinking about life-cycle management. The maximum number is limited by the maximum of the number of interrupt specifiers in the device tree, which are all static values. > But, as you wrote, INTRNG code is "absolutely free to map and dispatch > multiple virtual IRQs from the same shared interrupt pin" (allow me to > expand this also to INTRNG code itself) . > In attempt to simply situation abound cache, in first step, we removed > duplicates check from above model. This also removes unpleasantly > sharing of entries. > This step converts cache to some sort of duplicate of resource list > entries. Each existing RLE have exactly one corresponding entry in cache. > Also, the cache is not needed for INTRNG core. At this point, its only > transfers data from ofw_bus_intr_to_rl() to consumers PIC. > > So, if we are able to attach 'key' to RLE, then we can remove the whole > cache machinery from INTRNG. > Do you agree? No. Because you have to support two things that this still can't allow: 1. Devices attaching before their interrupt controllers. 2. Interrupts encoded entirely in a 32-bit integer rather than a struct resource * in a shared rman. This is required to support PCI (both LSI and MSI). #2 could of course be fixed by completely retooling the API of the PCI bus code, but, since it fixes something that currently is not a problem, why would we do that? > >>> For example: >>> ------------------------- >>> foo1: bar@12345678 { >>> interrupt-parent = <&pic1>; >>> interrupts = <1 IRQ_TYPE_LEVEL_HIGH>; >>> } >>> >>> foo2: bar@1234567C { >>> interrupt-parent = <&pic1>; >>> interrupts = <1 IRQ_TYPE_NONE>; >>> } >>> >>> Is this valid (from DT point of view) configuration? I think that yes. >> No, it's malformed and a violation of the standard. > Can you give me pointer, please? I'm not able to find anything about > this... I will try to find the reference -- unfortunately these things are scattered about and not all of the documents are public. You can see that this must be invalid immediately, however, because it would make the interrupt configuration dependent on which devices attach and in which order. I've attached the main interrupt mapping spec if you would like to look at it. > >>> Is this frequent configuration? I'm sure, isn't. >>> Is this possible configuration? I'm afraid that yes. >> People do in fact do deranged non-conformant things with device trees >> sometimes, unfortunately, so it is indeed worth worrying about. >> >>> -------------------------- >>> >>> Next example are GPIO interrupts. These are encoded differently, so two >>> 'keys' may point to to same real IRQ. >>> After that we decided to change our approach and utilize standard >>> resources and resource list entries to deliveryconfiguration data from >>> various sources (OFW/GPIO/...) to consumers. >> This is a non-issue. You make the GPIO controller a PIC, it handles >> interrupts however you like. If you got a weird device tree that uses >> two encodings (one for "interrupts" properties with GPIOs and one for >> "GPIO" properties on which you need to take interrupts but that >> weren't added to the "interrupts" list for mysterious reasons), you >> introduce a helper function for the GPIO case. >> >> Are there any actual problems with the pre-existing interrupt mapping >> code? I have not seen any so far. >> >>> [2] >>> Each OFW device has a lot of dependencies. It must enable clocks, power >>> regulators..., it must be taken from reset. All these action must be >>> done before we can access single register. >>> Most of these action can be done only with attached clock/power/reset >>> controller. >>> Within this, interrupts are not special. >> They are actually quite special in that the kernel enables them late >> and so you can defer setup. They are also special in that, for that >> reason, it is possible to design systems with circular dependency >> graphs with interrupts. It is not possible to do this with, for >> example, clocks: if I need to apply a clock to the clock controller >> with itself, the system is just not bootable and such a system will >> not be built or shipped. Interrupts need only happen later, after >> device setup, and so such systems *are* designed and shipped. The same >> is true for power. > The G5 problem is standard 'cross type' circular dependency. You must > (at BUS_PASS_BUS) initialize memory address window of PCI bridge and > start bus enumeration. > Rest of PCI bridge initialization (including all IRQ stuff) must be > postponed to any later pass. Of course, all interrupt controllers must > be attached at BUS_PASS_INTERRUPT. > Or I missed something? Yes. As I said, you could solve it that way. It would, of course, require a massive retooling of newbus (to allow partial delayed attach), everything in dev/pci (which doesn't expect that), and (almost) every PCI driver in and out of the tree. Or: you could skip all of that and use the fully functional mechanism that already exists. This is my point in a nutshell, basically. This code reinvents a perfectly functional wheel in a way that could, in the future, with a huge amount of work, accomplish the same things. For now, it is less functional and offers no visible advantages or new capabilities of any kind -- nor are there any such capabilities it could obviously provide in the future. Between now and that future, which will likely never arrive, we will have to maintain both in parallel and cause the device tree support on ARM and PowerPC to diverge. -Nathan >>> Moreover, 'cross type' circular >>> dependencies are not that rare. >>> I want to say: >>> Resource dependencies are must solved (and resolved) at different level >>> than is INTRNG. >>> Blind resource allocation is not universal solution because given >>> resource may/must be accessible immediately after allocation (in many >>> cases). >> Absolutely: for GPIOs, clocks, power, etc. you want a system that >> looks different than the interrupt virtualization system. Probably >> with extra resource types and maybe with some API similar to r301453. >> But interrupts have different, and more complex, requirements and >> cannot be mapped this way. >> >>> Unfortunately, and at time time, I known only one really working >>> solution: >>> staggered driver initialization combined with multipass bus >>> initialization. >> That does not solve the interrupt problem. If devices have interrupts >> on their own children, no amount of multipass initialization can >> possibly break the loop. Multipass would work for other kinds of >> resources (clocks, power, etc.) and is a perfect match for those >> resource types. A dynamic multipass (where a driver can return EAGAIN >> or something from attach if its resources don't exist yet) would work >> great. >> >> *But* in the specific, special case of interrupts, it is not a >> workable model. You could imagine some change to initialization that >> gives devices a late attach and an early attach method and does >> interrupts in the late part, but that is hugely invasive change that >> would need to touch every single driver in the tree -- to solve a >> problem that is already solved by interrupt virtualization. >> -Nathan >> >>> I'm sorry, I'm not able to express all this accurately and clearly, but >>> I've really tried... >>> Michal >>> >>> >>> _______________________________________________ >>> freebsd-arch@freebsd.org mailing list >>> https://lists.freebsd.org/mailman/listinfo/freebsd-arch >>> To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" >>> > _______________________________________________ > freebsd-arch@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/freebsd-arch > To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" > From owner-freebsd-arch@freebsd.org Fri Jul 29 07:03:42 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 4D14CBA6D4B; Fri, 29 Jul 2016 07:03:42 +0000 (UTC) (envelope-from mmel@FreeBSD.org) Received: from mail.miracle.cz (mail.miracle.cz [193.84.128.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mail.miracle.cz", Issuer "Miracle Group Root CA" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id E2F3B1D3D; Fri, 29 Jul 2016 07:03:41 +0000 (UTC) (envelope-from mmel@FreeBSD.org) Received: from [193.84.128.50] (meloun.ad.miracle.cz [193.84.128.50]) by mail.miracle.cz (Postfix) with ESMTPSA id 781CD3ACF0; Fri, 29 Jul 2016 09:03:33 +0200 (CEST) Subject: Re: INTRNG (Was: svn commit: r301453....) To: Nathan Whitehorn References: <201606051620.u55GKD5S066398@repo.freebsd.org> <57907B0F.9070204@FreeBSD.org> <9d2a224c-b787-2875-5984-a7a2354e8695@freebsd.org> <57934ABD.6010807@FreeBSD.org> <4e7a3e8f-cc21-f5f2-e3e0-4dbd554a4cd0@freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <57961549.4020105@FreeBSD.org> <57976867.6080705@FreeBSD.org> <5798E104.5020104@FreeBSD.org> <579A25BB.8070206@FreeBSD.org> <30790e40-58b4-3371-c0f0-b7545571f389@freebsd.org> Cc: "freebsd-arm@freebsd.org" , Svatopluk Kraus , "freebsd-arch@freebsd.org" From: Michal Meloun X-Enigmail-Draft-Status: N1110 Message-ID: <579AFFC5.1040005@FreeBSD.org> Date: Fri, 29 Jul 2016 09:03:33 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <30790e40-58b4-3371-c0f0-b7545571f389@freebsd.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.4.3 (mail.miracle.cz); Fri, 29 Jul 2016 09:03:33 +0200 (CEST) X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Jul 2016 07:03:42 -0000 Dne 28.07.2016 v 19:23 Nathan Whitehorn napsal(a): > > > On 07/28/16 08:33, Michal Meloun wrote: >> Dne 27.07.2016 v 19:19 Nathan Whitehorn napsal(a): >>> >>> On 07/27/16 09:27, Michal Meloun wrote: >>> >>> >>> [snip] >>>>> The concept is *extremely* necessary, which is why both Linux and >>>>> FreeBSD decided to use it independently There is no way to handle >>>>> parent buses with a single rman and devices on multiple PICs with >>>>> overlapping interrupt ranges without them; neither is there a way to >>>>> decode arbitrary-length interrupt specifiers or to handle things like >>>>> MSIs. Please see the list of cases at >>>>> https://wiki.freebsd.org/Complicated_Interrupts and in my earlier >>>>> email for some examples of things that you just can't represent with >>>>> this new system, as far as I can tell. >>>>> >>>>>> Also, both variants needs attached PIC at bus_alloc_resource() >>>>>> time, >>>>>> so timing wasn't been changed. >>>>> They absolutely do not, as I have explained repeatedly. Due to parent >>>>> devices with interrupts handled by their bus children, this is a hard >>>>> requirement of any workable system. This is not a theoretical >>>>> issue; I >>>>> have lots of hardware like this. >>>> Nathan, I'm talking about pre /post r301453 state. Current >>>> INTRNG it >>>> has never supported. But yes, I'm fully understand why you want >>>> it. See >>>> [2]. >>> Current "INTRNG", at least on PowerPC, has supported this for 10 >>> years. If it doesn't on ARM, it's because of some trivial >>> implementation bug. >>>>>> - it implements new BUS_MAP_INTR(). As I understand it, this is >>>>>> problematic for you, and I'm ready to change it. But I need more >>>>>> details >>>>>> than "it's fundamentally broken". >>>>> Please explain (a) what cases it handles that the existing code and >>>>> does, and (b) how you would resolve each of the cases on the wiki >>>>> page >>>>> I sent. >>>>> >>>>> The general issue is that it traces the newbus hierarchy, when >>>>> interrupts often do not, and so breaks when you have links to >>>>> as-yet-unattached parts of the hierarchy. It also relies on the >>>>> assigned "IRQ" numbers being nonoverlapping to avoid rman errors, but >>>>> that's a relatively minor thing. >>>> Well, from my point of view, only "get interrupt data from >>>> OFW/GPIO/.." part of bus_alloc_resource() follows bus hierarchy, >>>> real >>>> execution not changed. >>>> In any case, I see many possible variants how we can modify current >>>> implementation. For rest, see [2]. >>> But that's an important part, for three reasons: >>> 1. The PIC may not exist when bus_alloc_resource() is called >>> 2. The bus parents have no useful information to contribute to this >>> since the hierarchy doesn't go that way >> See [2] >> >>> 3. If you try to use the interrupt pin as the resource ID, which >>> r301453 does, you end up with rman conflicts if, for example, a bus >>> has two children with interrupts on identically numbered pins on two >>> different interrupt controllers. >> No, r301453 doesn't do this. >> Index to global interrupt source table (irq_sources) is used as resource >> ID (and r301453 not changed it). > > Ah, I see that now. Apologies for the confusion. > > So, to understand fully: your major complaint about the existing code > is that it uses arbitrary numbers reflecting some table lookup value > as IRQ numbers, which r301453 *also* does? No, my major complaint is that MIPS and pre-r301453 INTRNG mixes data from independent sources in one table (irq_sources) . Which r301453 eliminates. All what I want is to have data associated to RLE separated from data associated to already allocated (by bus_alloc_resouce()) interrupts. >>>>>> >>>>>>> There are some other differences, of varying degrees of importance, >>>>>>> but that's the really fundamental one. I haven't seen any cases >>>>>>> where >>>>>>> r301453 provides functionality absent in the already existing >>>>>>> system, >>>>>>> but there seem to be a large number of cases (see the first email I >>>>>>> sent to freebsd-arch or >>>>>>> https://wiki.freebsd.org/Complicated_Interrupts) that the API in >>>>>>> r301453 cannot accommodate and that are needed to support a >>>>>>> variety of >>>>>>> hardware. >>>>>> Which single feature has been removed by r301453? >>>>> Nothing has been removed by it, because the normal code is intact. >>>>> However, the new code is less functional than the old code, so cannot >>>>> replace it. In particular, it is architecturally incapable of working >>>>> with the kinds of device trees found on PowerPC systems (see the wiki >>>>> page). That means we would have to keep both indefinitely, which is a >>>>> significant maintenance burden to no gain whatsoever. >>>>> -Nathan >>>> Firstly, please, ignore dependency problem. It will be explained >>>> later >>>> in [2]. >>> Except that that isn't a workable solution (see the end) >>> >>>> [1] >>>> The original (your) INTRNG assume several things: >>>> >>>> - value of interrupt parent xref together with byte contents of >>>> 'interrupts' property forms some sort of 'key' (aka virtual IRQ) >>>> - byte contents of 'interrupts' property cannot be parsed in any way >>>> inside INTRNG core. >>>> - data forming this key are sufficient for subsequent mapping (and/or >>>> configuring) to real IRQ. >>>> - there must exist bidirectional unique relation between virtual and >>>> real IRQs. So one key (virtual IRQ) can be mappable to one and just >>>> one >>>> real IRQ. And only one virtual IRQ can be mapped to any >>>> given real >>>> IRQ. >>>> - we have cache that stores all observed 'keys' and associated real >>>> IRQ >>>> ) the they already mapped. >>>> >>>> Unfortunately, necessity of unique mapping is very problematic. >>>> We cannot handle situation, where shared interrupt is declared by >>>> different but compatible 'interrupts' properties. >>> There isn't actually a requirement of bidirectional uniqueness, as I >>> explained the last time you brought this up. One "virtual" IRQ does >>> need to map to exactly one interrupt specifier, but the reverse is not >>> the case. The PIC driver is absolutely free to map and dispatch >>> multiple virtual IRQs from the same shared interrupt pin, with no more >>> overhead than you usually get from shared interrupt lines. >> Oki, lets me to expand this a little bit more. >> >> The pre r301453 INTRNG[ARM] does this: >> - ofw_bus_intr_to_rl() reads and pre-parses 'interrupts' property >> into 'key'. > > Sometimes. For some buses. ofw_bus_intr_to_rl() is a function of very > limited application that should really only be used by simplebus; how > that works depends on the bus bindings. There are many cases (e.g. > MSIs, PCI buses generally) where interrupts are set up by a different > route. Yes, I agree. >> - cache is searched for duplicates and new entry is created. >> - position of this entry in cache is used as resource ID. > > Resource ID is arbitrary, but it could be this. > >> - given resource ID is stored to RL and is later used in >> bus_alloc_resource() > Yes. >> I'm right ? >> >> In the above model, individual cache entries may be shared are >> referenced by multiple entities (PIC, RL, ..). Due to this, life cycle >> of cache entries is not well defined and we probably must use some sort >> of reference counting to control it. > > Why? Interrupts are not unmapped and remapped with unique values often > enough (or, really, at all) to warrant even thinking about life-cycle > management. The maximum number is limited by the maximum of the number > of interrupt specifiers in the device tree, which are all static values. No, we must also support attachable/removable devices. GPIO board attached to hot-pug capable interfaces(USB, ExpressCard,...) for example. Due to this we must be able to attach/detach PIC at run-time. > >> But, as you wrote, INTRNG code is "absolutely free to map and dispatch >> multiple virtual IRQs from the same shared interrupt pin" (allow me to >> expand this also to INTRNG code itself) . >> In attempt to simply situation abound cache, in first step, we removed >> duplicates check from above model. This also removes unpleasantly >> sharing of entries. >> This step converts cache to some sort of duplicate of resource list >> entries. Each existing RLE have exactly one corresponding entry in >> cache. >> Also, the cache is not needed for INTRNG core. At this point, its only >> transfers data from ofw_bus_intr_to_rl() to consumers PIC. >> >> So, if we are able to attach 'key' to RLE, then we can remove the whole >> cache machinery from INTRNG. >> Do you agree? > > No. Because you have to support two things that this still can't allow: > 1. Devices attaching before their interrupt controllers. You still expect that data readed from OFW must be parsed at RLE creation time. I don't see single reason why we cannot postpone parsing to bus_alloc_resource() time. At this point, its irrelevant if bus_alloc_resource() supports detached PIC or not. > 2. Interrupts encoded entirely in a 32-bit integer rather than a > struct resource * in a shared rman. This is required to support PCI > (both LSI and MSI). > > #2 could of course be fixed by completely retooling the API of the PCI > bus code, but, since it fixes something that currently is not a > problem, why would we do that? All relevant bus functions have struct resource * as an argument. And, in post r301453 kernel, all necessary data are attached to it. I don't see any reason for API change. > >> >>>> For example: >>>> ------------------------- >>>> foo1: bar@12345678 { >>>> interrupt-parent = <&pic1>; >>>> interrupts = <1 IRQ_TYPE_LEVEL_HIGH>; >>>> } >>>> >>>> foo2: bar@1234567C { >>>> interrupt-parent = <&pic1>; >>>> interrupts = <1 IRQ_TYPE_NONE>; >>>> } >>>> >>>> Is this valid (from DT point of view) configuration? I think that >>>> yes. >>> No, it's malformed and a violation of the standard. >> Can you give me pointer, please? I'm not able to find anything about >> this... > > I will try to find the reference -- unfortunately these things are > scattered about and not all of the documents are public. Don't worry, I trust you :) > You can see that this must be invalid immediately, however, because it > would make the interrupt configuration dependent on which devices > attach and in which order. I've attached the main interrupt mapping > spec if you would like to look at it. Well, if IRQ_TYPE_LEVEL_HIGH is default interrupt configuration then not. And with current hell with various shield boards and its overlays... >> >>>> Is this frequent configuration? I'm sure, isn't. >>>> Is this possible configuration? I'm afraid that yes. >>> People do in fact do deranged non-conformant things with device trees >>> sometimes, unfortunately, so it is indeed worth worrying about. >>> >>>> -------------------------- >>>> >>>> Next example are GPIO interrupts. These are encoded differently, >>>> so two >>>> 'keys' may point to to same real IRQ. >>>> After that we decided to change our approach and utilize standard >>>> resources and resource list entries to deliveryconfiguration data from >>>> various sources (OFW/GPIO/...) to consumers. >>> This is a non-issue. You make the GPIO controller a PIC, it handles >>> interrupts however you like. If you got a weird device tree that uses >>> two encodings (one for "interrupts" properties with GPIOs and one for >>> "GPIO" properties on which you need to take interrupts but that >>> weren't added to the "interrupts" list for mysterious reasons), you >>> introduce a helper function for the GPIO case. >>> >>> Are there any actual problems with the pre-existing interrupt mapping >>> code? I have not seen any so far. >>> >>>> [2] >>>> Each OFW device has a lot of dependencies. It must enable clocks, >>>> power >>>> regulators..., it must be taken from reset. All these action must be >>>> done before we can access single register. >>>> Most of these action can be done only with attached clock/power/reset >>>> controller. >>>> Within this, interrupts are not special. >>> They are actually quite special in that the kernel enables them late >>> and so you can defer setup. They are also special in that, for that >>> reason, it is possible to design systems with circular dependency >>> graphs with interrupts. It is not possible to do this with, for >>> example, clocks: if I need to apply a clock to the clock controller >>> with itself, the system is just not bootable and such a system will >>> not be built or shipped. Interrupts need only happen later, after >>> device setup, and so such systems *are* designed and shipped. The same >>> is true for power. >> The G5 problem is standard 'cross type' circular dependency. You must >> (at BUS_PASS_BUS) initialize memory address window of PCI bridge and >> start bus enumeration. >> Rest of PCI bridge initialization (including all IRQ stuff) must be >> postponed to any later pass. Of course, all interrupt controllers must >> be attached at BUS_PASS_INTERRUPT. >> Or I missed something? > > Yes. As I said, you could solve it that way. It would, of course, > require a massive retooling of newbus (to allow partial delayed > attach), everything in dev/pci (which doesn't expect that), and > (almost) every PCI driver in and out of the tree. I still think that only host to PCI bridge must me modified, and PCI-PCI bridge must be run at BUS_PASS_BUS (which is single line modification). > > Or: you could skip all of that and use the fully functional mechanism > that already exists. The problem is, at least from my point of view, that we don't have it. The actual MIPS code doesn't work on ARM for numerous reasons. This is first one -> https://svnweb.freebsd.org/base/head/sys/powerpc/powerpc/nexus.c?revision=297000&view=markup#l186 The pre-r301453 ARM INTRNG doesn't support detached interrupts. And r301453 doesn't changed nothing about this. Michal > This is my point in a nutshell, basically. This code reinvents a > perfectly functional wheel in a way that could, in the future, with a > huge amount of work, accomplish the same things. For now, it is less > functional and offers no visible advantages or new capabilities of any > kind -- nor are there any such capabilities it could obviously provide > in the future. Between now and that future, which will likely never > arrive, we will have to maintain both in parallel and cause the device > tree support on ARM and PowerPC to diverge. > -Nathan > >>>> Moreover, 'cross type' circular >>>> dependencies are not that rare. >>>> I want to say: >>>> Resource dependencies are must solved (and resolved) at different >>>> level >>>> than is INTRNG. >>>> Blind resource allocation is not universal solution because given >>>> resource may/must be accessible immediately after allocation (in many >>>> cases). >>> Absolutely: for GPIOs, clocks, power, etc. you want a system that >>> looks different than the interrupt virtualization system. Probably >>> with extra resource types and maybe with some API similar to r301453. >>> But interrupts have different, and more complex, requirements and >>> cannot be mapped this way. >>> >>>> Unfortunately, and at time time, I known only one really working >>>> solution: >>>> staggered driver initialization combined with multipass bus >>>> initialization. >>> That does not solve the interrupt problem. If devices have interrupts >>> on their own children, no amount of multipass initialization can >>> possibly break the loop. Multipass would work for other kinds of >>> resources (clocks, power, etc.) and is a perfect match for those >>> resource types. A dynamic multipass (where a driver can return EAGAIN >>> or something from attach if its resources don't exist yet) would work >>> great. >>> >>> *But* in the specific, special case of interrupts, it is not a >>> workable model. You could imagine some change to initialization that >>> gives devices a late attach and an early attach method and does >>> interrupts in the late part, but that is hugely invasive change that >>> would need to touch every single driver in the tree -- to solve a >>> problem that is already solved by interrupt virtualization. >>> -Nathan >>> >>>> I'm sorry, I'm not able to express all this accurately and clearly, >>>> but >>>> I've really tried... >>>> Michal >>>> >>>> >>>> _______________________________________________ >>>> freebsd-arch@freebsd.org mailing list >>>> https://lists.freebsd.org/mailman/listinfo/freebsd-arch >>>> To unsubscribe, send any mail to >>>> "freebsd-arch-unsubscribe@freebsd.org" >>>> >> _______________________________________________ >> freebsd-arch@freebsd.org mailing list >> https://lists.freebsd.org/mailman/listinfo/freebsd-arch >> To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" >> > From owner-freebsd-arch@freebsd.org Fri Jul 29 14:41:32 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 2BD16BA74F5; Fri, 29 Jul 2016 14:41:32 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from d.mail.sonic.net (d.mail.sonic.net [64.142.111.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 114C1135A; Fri, 29 Jul 2016 14:41:31 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from zeppelin.tachypleus.net (75-101-50-44.static.sonic.net [75.101.50.44]) (authenticated bits=0) by d.mail.sonic.net (8.15.1/8.15.1) with ESMTPSA id u6TEfMW1014844 (version=TLSv1.2 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Fri, 29 Jul 2016 07:41:22 -0700 Subject: Re: INTRNG (Was: svn commit: r301453....) To: Michal Meloun References: <201606051620.u55GKD5S066398@repo.freebsd.org> <57907B0F.9070204@FreeBSD.org> <9d2a224c-b787-2875-5984-a7a2354e8695@freebsd.org> <57934ABD.6010807@FreeBSD.org> <4e7a3e8f-cc21-f5f2-e3e0-4dbd554a4cd0@freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <57961549.4020105@FreeBSD.org> <57976867.6080705@FreeBSD.org> <5798E104.5020104@FreeBSD.org> <579A25BB.8070206@FreeBSD.org> <30790e40-58b4-3371-c0f0-b7545571f389@freebsd.org> <579AFFC5.1040005@FreeBSD.org> Cc: "freebsd-arm@freebsd.org" , Svatopluk Kraus , "freebsd-arch@freebsd.org" From: Nathan Whitehorn Message-ID: Date: Fri, 29 Jul 2016 07:41:22 -0700 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:45.0) Gecko/20100101 Thunderbird/45.1.0 MIME-Version: 1.0 In-Reply-To: <579AFFC5.1040005@FreeBSD.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Sonic-CAuth: UmFuZG9tSVagc1QqOjDoKcjiAOycMFsq9P5oNOKeSVIPQqgGl3SzQ6ACkveVsPokOKkjrdqa9byoYMxgMwsv+uWmGcHpXMkQOMNW1Mc7TBs= X-Sonic-ID: C;aArRhJpV5hGZNpNwxPCmMQ== M;2t8QhZpV5hGZNpNwxPCmMQ== X-Spam-Flag: No X-Sonic-Spam-Details: 0.0/5.0 by cerberusd X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Jul 2016 14:41:32 -0000 On 07/29/16 00:03, Michal Meloun wrote: > Dne 28.07.2016 v 19:23 Nathan Whitehorn napsal(a): >> >> On 07/28/16 08:33, Michal Meloun wrote: >>> Dne 27.07.2016 v 19:19 Nathan Whitehorn napsal(a): >>>> On 07/27/16 09:27, Michal Meloun wrote: >>>> >>>> >>>> [snip] >>>>>> The concept is *extremely* necessary, which is why both Linux and >>>>>> FreeBSD decided to use it independently There is no way to handle >>>>>> parent buses with a single rman and devices on multiple PICs with >>>>>> overlapping interrupt ranges without them; neither is there a way to >>>>>> decode arbitrary-length interrupt specifiers or to handle things like >>>>>> MSIs. Please see the list of cases at >>>>>> https://wiki.freebsd.org/Complicated_Interrupts and in my earlier >>>>>> email for some examples of things that you just can't represent with >>>>>> this new system, as far as I can tell. >>>>>> >>>>>>> Also, both variants needs attached PIC at bus_alloc_resource() >>>>>>> time, >>>>>>> so timing wasn't been changed. >>>>>> They absolutely do not, as I have explained repeatedly. Due to parent >>>>>> devices with interrupts handled by their bus children, this is a hard >>>>>> requirement of any workable system. This is not a theoretical >>>>>> issue; I >>>>>> have lots of hardware like this. >>>>> Nathan, I'm talking about pre /post r301453 state. Current >>>>> INTRNG it >>>>> has never supported. But yes, I'm fully understand why you want >>>>> it. See >>>>> [2]. >>>> Current "INTRNG", at least on PowerPC, has supported this for 10 >>>> years. If it doesn't on ARM, it's because of some trivial >>>> implementation bug. >>>>>>> - it implements new BUS_MAP_INTR(). As I understand it, this is >>>>>>> problematic for you, and I'm ready to change it. But I need more >>>>>>> details >>>>>>> than "it's fundamentally broken". >>>>>> Please explain (a) what cases it handles that the existing code and >>>>>> does, and (b) how you would resolve each of the cases on the wiki >>>>>> page >>>>>> I sent. >>>>>> >>>>>> The general issue is that it traces the newbus hierarchy, when >>>>>> interrupts often do not, and so breaks when you have links to >>>>>> as-yet-unattached parts of the hierarchy. It also relies on the >>>>>> assigned "IRQ" numbers being nonoverlapping to avoid rman errors, but >>>>>> that's a relatively minor thing. >>>>> Well, from my point of view, only "get interrupt data from >>>>> OFW/GPIO/.." part of bus_alloc_resource() follows bus hierarchy, >>>>> real >>>>> execution not changed. >>>>> In any case, I see many possible variants how we can modify current >>>>> implementation. For rest, see [2]. >>>> But that's an important part, for three reasons: >>>> 1. The PIC may not exist when bus_alloc_resource() is called >>>> 2. The bus parents have no useful information to contribute to this >>>> since the hierarchy doesn't go that way >>> See [2] >>> >>>> 3. If you try to use the interrupt pin as the resource ID, which >>>> r301453 does, you end up with rman conflicts if, for example, a bus >>>> has two children with interrupts on identically numbered pins on two >>>> different interrupt controllers. >>> No, r301453 doesn't do this. >>> Index to global interrupt source table (irq_sources) is used as resource >>> ID (and r301453 not changed it). >> Ah, I see that now. Apologies for the confusion. >> >> So, to understand fully: your major complaint about the existing code >> is that it uses arbitrary numbers reflecting some table lookup value >> as IRQ numbers, which r301453 *also* does? > No, my major complaint is that MIPS and pre-r301453 INTRNG mixes data > from independent sources in one table (irq_sources) . Which r301453 > eliminates. > All what I want is to have data associated to RLE separated from data > associated to already allocated (by bus_alloc_resouce()) interrupts. So the complaint is that pre-r301453 stores the raw interrupt specifier in the table and post-r301453 stores a decoded version of the interrupt specifier in the table? >>>>>>> >>>>>>>> There are some other differences, of varying degrees of importance, >>>>>>>> but that's the really fundamental one. I haven't seen any cases >>>>>>>> where >>>>>>>> r301453 provides functionality absent in the already existing >>>>>>>> system, >>>>>>>> but there seem to be a large number of cases (see the first email I >>>>>>>> sent to freebsd-arch or >>>>>>>> https://wiki.freebsd.org/Complicated_Interrupts) that the API in >>>>>>>> r301453 cannot accommodate and that are needed to support a >>>>>>>> variety of >>>>>>>> hardware. >>>>>>> Which single feature has been removed by r301453? >>>>>> Nothing has been removed by it, because the normal code is intact. >>>>>> However, the new code is less functional than the old code, so cannot >>>>>> replace it. In particular, it is architecturally incapable of working >>>>>> with the kinds of device trees found on PowerPC systems (see the wiki >>>>>> page). That means we would have to keep both indefinitely, which is a >>>>>> significant maintenance burden to no gain whatsoever. >>>>>> -Nathan >>>>> Firstly, please, ignore dependency problem. It will be explained >>>>> later >>>>> in [2]. >>>> Except that that isn't a workable solution (see the end) >>>> >>>>> [1] >>>>> The original (your) INTRNG assume several things: >>>>> >>>>> - value of interrupt parent xref together with byte contents of >>>>> 'interrupts' property forms some sort of 'key' (aka virtual IRQ) >>>>> - byte contents of 'interrupts' property cannot be parsed in any way >>>>> inside INTRNG core. >>>>> - data forming this key are sufficient for subsequent mapping (and/or >>>>> configuring) to real IRQ. >>>>> - there must exist bidirectional unique relation between virtual and >>>>> real IRQs. So one key (virtual IRQ) can be mappable to one and just >>>>> one >>>>> real IRQ. And only one virtual IRQ can be mapped to any >>>>> given real >>>>> IRQ. >>>>> - we have cache that stores all observed 'keys' and associated real >>>>> IRQ >>>>> ) the they already mapped. >>>>> >>>>> Unfortunately, necessity of unique mapping is very problematic. >>>>> We cannot handle situation, where shared interrupt is declared by >>>>> different but compatible 'interrupts' properties. >>>> There isn't actually a requirement of bidirectional uniqueness, as I >>>> explained the last time you brought this up. One "virtual" IRQ does >>>> need to map to exactly one interrupt specifier, but the reverse is not >>>> the case. The PIC driver is absolutely free to map and dispatch >>>> multiple virtual IRQs from the same shared interrupt pin, with no more >>>> overhead than you usually get from shared interrupt lines. >>> Oki, lets me to expand this a little bit more. >>> >>> The pre r301453 INTRNG[ARM] does this: >>> - ofw_bus_intr_to_rl() reads and pre-parses 'interrupts' property >>> into 'key'. >> Sometimes. For some buses. ofw_bus_intr_to_rl() is a function of very >> limited application that should really only be used by simplebus; how >> that works depends on the bus bindings. There are many cases (e.g. >> MSIs, PCI buses generally) where interrupts are set up by a different >> route. > Yes, I agree. >>> - cache is searched for duplicates and new entry is created. >>> - position of this entry in cache is used as resource ID. >> Resource ID is arbitrary, but it could be this. >> >>> - given resource ID is stored to RL and is later used in >>> bus_alloc_resource() >> Yes. >>> I'm right ? >>> >>> In the above model, individual cache entries may be shared are >>> referenced by multiple entities (PIC, RL, ..). Due to this, life cycle >>> of cache entries is not well defined and we probably must use some sort >>> of reference counting to control it. >> Why? Interrupts are not unmapped and remapped with unique values often >> enough (or, really, at all) to warrant even thinking about life-cycle >> management. The maximum number is limited by the maximum of the number >> of interrupt specifiers in the device tree, which are all static values. > No, we must also support attachable/removable devices. GPIO board > attached to hot-pug capable interfaces(USB, ExpressCard,...) for example. > Due to this we must be able to attach/detach PIC at run-time. OK... There just really aren't that many cases of this, but fine. If you find yourself needing hot-pluggable PICs, neither API is any more or less complicated than the other. You can't use FDT entries for this, since new devices aren't in the FDT, and you presumably end up in a much more packaged situation where only the things on the hotplug card depend on the PIC on the hotplug device, so that's nice. In either case, you just have to clean up mappings to the associated PIC when the PIC detaches (and would, in both cases, have some assert that all IRQs handled by that PIC have been deallocated first before cleaning up). But it's just not that bad. >>> But, as you wrote, INTRNG code is "absolutely free to map and dispatch >>> multiple virtual IRQs from the same shared interrupt pin" (allow me to >>> expand this also to INTRNG code itself) . >>> In attempt to simply situation abound cache, in first step, we removed >>> duplicates check from above model. This also removes unpleasantly >>> sharing of entries. >>> This step converts cache to some sort of duplicate of resource list >>> entries. Each existing RLE have exactly one corresponding entry in >>> cache. >>> Also, the cache is not needed for INTRNG core. At this point, its only >>> transfers data from ofw_bus_intr_to_rl() to consumers PIC. >>> >>> So, if we are able to attach 'key' to RLE, then we can remove the whole >>> cache machinery from INTRNG. >>> Do you agree? >> No. Because you have to support two things that this still can't allow: >> 1. Devices attaching before their interrupt controllers. > You still expect that data readed from OFW must be parsed at RLE > creation time. I don't see single reason why we cannot postpone > parsing to bus_alloc_resource() time. > At this point, its irrelevant if bus_alloc_resource() supports > detached PIC or not. Because there are *lots* of cases in the kernel where interrupts are represented as a number and not a struct resource *. I've listed many of them; I'm sure there are more. PCI is the most notable example (both LSI and MSI). For both of these, the route_interrupt (LSI) and alloc_msi[x]() (MSI) functions give the bus driver a chance to look at the device tree and then they return an IRQ number. There doesn't seem to be any way to map that back to a struct resource * or something besides an internal table. Moreover, there are cases (self-added interrupts by children, for instance) in which the RL assigned by the parent does not reflect the final RL used by the child and there is no way for the parent bus to map an RID to a device tree entry. When we moved to doing this at interrupt assignment time rather than resource allocation time or another occasion on PowerPC, it was for these kinds of reasons. There were ways to do it later, but they were hugely invasive and involved changing large parts of the kernel. Even then, it would still be fragile: Trying to guess what device tree entry was meant at bus_alloc_resource() time is much more error-prone than doing the mapping when reading that device tree to assign the resources in the first place, when you can make guarantees. > >> 2. Interrupts encoded entirely in a 32-bit integer rather than a >> struct resource * in a shared rman. This is required to support PCI >> (both LSI and MSI). >> >> #2 could of course be fixed by completely retooling the API of the PCI >> bus code, but, since it fixes something that currently is not a >> problem, why would we do that? > All relevant bus functions have struct resource * as an argument. And, > in post r301453 kernel, all necessary data are attached to it. > I don't see any reason for API change. There is: int PCIB_ROUTE_INTERRUPT(device_t, device_t, pin) Which takes a PIN (A, B, C, D) and returns the IRQ to add to the RL. This is the only real opportunity to parse an interrupt-map and does not deal with struct resource. There are also PCIB_ALLOC_MSI[X], which take a device_t and count and return a vector of IRQ numbers. How would you do resource allocation given these APIs? There seem to be some dubious-looking attempts at generic MSI mapping code in the new kern/subr_intr.c that don't allow the specification of more than 32-bit interrupt specifiers and otherwise exactly duplicate the pre-301453 flow for MSIs, but in a more complicated and less-functional way. (While investigating this, I also just found dev/pci/pci_host_generic.c, which mostly does the wrong things, is actually specific to a couple of ARM boards, and duplicates code in dev/ofw/ofwpci.c. But that's another discussion...) > >>>>> For example: >>>>> ------------------------- >>>>> foo1: bar@12345678 { >>>>> interrupt-parent = <&pic1>; >>>>> interrupts = <1 IRQ_TYPE_LEVEL_HIGH>; >>>>> } >>>>> >>>>> foo2: bar@1234567C { >>>>> interrupt-parent = <&pic1>; >>>>> interrupts = <1 IRQ_TYPE_NONE>; >>>>> } >>>>> >>>>> Is this valid (from DT point of view) configuration? I think that >>>>> yes. >>>> No, it's malformed and a violation of the standard. >>> Can you give me pointer, please? I'm not able to find anything about >>> this... >> I will try to find the reference -- unfortunately these things are >> scattered about and not all of the documents are public. > Don't worry, I trust you :) > >> You can see that this must be invalid immediately, however, because it >> would make the interrupt configuration dependent on which devices >> attach and in which order. I've attached the main interrupt mapping >> spec if you would like to look at it. > Well, if IRQ_TYPE_LEVEL_HIGH is default interrupt configuration then > not. And with current hell with various shield boards and its overlays... As I said, people do stupid things. This is such a minor problem that it is best worried about when seen in the wild. > >>>>> Is this frequent configuration? I'm sure, isn't. >>>>> Is this possible configuration? I'm afraid that yes. >>>> People do in fact do deranged non-conformant things with device trees >>>> sometimes, unfortunately, so it is indeed worth worrying about. >>>> >>>>> -------------------------- >>>>> >>>>> Next example are GPIO interrupts. These are encoded differently, >>>>> so two >>>>> 'keys' may point to to same real IRQ. >>>>> After that we decided to change our approach and utilize standard >>>>> resources and resource list entries to deliveryconfiguration data from >>>>> various sources (OFW/GPIO/...) to consumers. >>>> This is a non-issue. You make the GPIO controller a PIC, it handles >>>> interrupts however you like. If you got a weird device tree that uses >>>> two encodings (one for "interrupts" properties with GPIOs and one for >>>> "GPIO" properties on which you need to take interrupts but that >>>> weren't added to the "interrupts" list for mysterious reasons), you >>>> introduce a helper function for the GPIO case. >>>> >>>> Are there any actual problems with the pre-existing interrupt mapping >>>> code? I have not seen any so far. >>>> >>>>> [2] >>>>> Each OFW device has a lot of dependencies. It must enable clocks, >>>>> power >>>>> regulators..., it must be taken from reset. All these action must be >>>>> done before we can access single register. >>>>> Most of these action can be done only with attached clock/power/reset >>>>> controller. >>>>> Within this, interrupts are not special. >>>> They are actually quite special in that the kernel enables them late >>>> and so you can defer setup. They are also special in that, for that >>>> reason, it is possible to design systems with circular dependency >>>> graphs with interrupts. It is not possible to do this with, for >>>> example, clocks: if I need to apply a clock to the clock controller >>>> with itself, the system is just not bootable and such a system will >>>> not be built or shipped. Interrupts need only happen later, after >>>> device setup, and so such systems *are* designed and shipped. The same >>>> is true for power. >>> The G5 problem is standard 'cross type' circular dependency. You must >>> (at BUS_PASS_BUS) initialize memory address window of PCI bridge and >>> start bus enumeration. >>> Rest of PCI bridge initialization (including all IRQ stuff) must be >>> postponed to any later pass. Of course, all interrupt controllers must >>> be attached at BUS_PASS_INTERRUPT. >>> Or I missed something? >> Yes. As I said, you could solve it that way. It would, of course, >> require a massive retooling of newbus (to allow partial delayed >> attach), everything in dev/pci (which doesn't expect that), and >> (almost) every PCI driver in and out of the tree. > I still think that only host to PCI bridge must me modified, and PCI-PCI > bridge must be run at BUS_PASS_BUS (which is single line modification). On Powermacs, one PIC is 5 levels deep in the bus hierarchy, under 3 PCI<->PCI bridges and a device that also has an ATA controller and a bunch of other things. Some of those can have their own interrupts (which is fairly common for error reporting or PCI-E hotplug). It wouldn't be impossible, but it is decidedly nontrivial. >> Or: you could skip all of that and use the fully functional mechanism >> that already exists. > The problem is, at least from my point of view, that we don't have it. > The actual MIPS code doesn't work on ARM for numerous reasons. This is > first one -> > > https://svnweb.freebsd.org/base/head/sys/powerpc/powerpc/nexus.c?revision=297000&view=markup#l186 > The pre-r301453 ARM INTRNG doesn't support detached interrupts. > And r301453 doesn't changed nothing about this. I'm happy to write whatever code is missing. The ARM implementation of ofw_bus_map_intr() does seem fairly braindead and should be replaced. So here is where I am right now: - The perceived advantage of the new API seems to be that the mapping information (interrupt parent, etc.) ends up in a struct resource instead of in a centralized mapping table - Additionally, it gives you a better shot at having the PIC online before the PIC's interrupts are parsed (which is not required, but nice). - Beyond these aesthetic points, there are no concrete examples of new functionality added by this API, aside from some minor implementation bugs of the old one on ARM that are easily fixed. - There are, conversely, a number of concrete cases where this new API would not be able to do the right thing. Some of these can be worked around or fixed with significant restructuring in the MI parts of the kernel. - If we have both, the interrupt handling code in the MI parts of the kernel will bifurcate. This patch alone has added a bunch of #ifdef to kern/subr_bus.c, a new file to kern/, and lots of #ifdef to dev/ofw. This is going to be really hard to maintain if we need both. Is this all correct? If so, can we please back this out until this discussion is complete? I'm asking this formally at this point, under the Committer's Guide section about reversion requests. -Nathan > Michal > >> This is my point in a nutshell, basically. This code reinvents a >> perfectly functional wheel in a way that could, in the future, with a >> huge amount of work, accomplish the same things. For now, it is less >> functional and offers no visible advantages or new capabilities of any >> kind -- nor are there any such capabilities it could obviously provide >> in the future. Between now and that future, which will likely never >> arrive, we will have to maintain both in parallel and cause the device >> tree support on ARM and PowerPC to diverge. >> -Nathan >> >>>>> Moreover, 'cross type' circular >>>>> dependencies are not that rare. >>>>> I want to say: >>>>> Resource dependencies are must solved (and resolved) at different >>>>> level >>>>> than is INTRNG. >>>>> Blind resource allocation is not universal solution because given >>>>> resource may/must be accessible immediately after allocation (in many >>>>> cases). >>>> Absolutely: for GPIOs, clocks, power, etc. you want a system that >>>> looks different than the interrupt virtualization system. Probably >>>> with extra resource types and maybe with some API similar to r301453. >>>> But interrupts have different, and more complex, requirements and >>>> cannot be mapped this way. >>>> >>>>> Unfortunately, and at time time, I known only one really working >>>>> solution: >>>>> staggered driver initialization combined with multipass bus >>>>> initialization. >>>> That does not solve the interrupt problem. If devices have interrupts >>>> on their own children, no amount of multipass initialization can >>>> possibly break the loop. Multipass would work for other kinds of >>>> resources (clocks, power, etc.) and is a perfect match for those >>>> resource types. A dynamic multipass (where a driver can return EAGAIN >>>> or something from attach if its resources don't exist yet) would work >>>> great. >>>> >>>> *But* in the specific, special case of interrupts, it is not a >>>> workable model. You could imagine some change to initialization that >>>> gives devices a late attach and an early attach method and does >>>> interrupts in the late part, but that is hugely invasive change that >>>> would need to touch every single driver in the tree -- to solve a >>>> problem that is already solved by interrupt virtualization. >>>> -Nathan >>>> >>>>> I'm sorry, I'm not able to express all this accurately and clearly, >>>>> but >>>>> I've really tried... >>>>> Michal >>>>> >>>>> >>>>> _______________________________________________ >>>>> freebsd-arch@freebsd.org mailing list >>>>> https://lists.freebsd.org/mailman/listinfo/freebsd-arch >>>>> To unsubscribe, send any mail to >>>>> "freebsd-arch-unsubscribe@freebsd.org" >>>>> >>> _______________________________________________ >>> freebsd-arch@freebsd.org mailing list >>> https://lists.freebsd.org/mailman/listinfo/freebsd-arch >>> To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" >>> From owner-freebsd-arch@freebsd.org Fri Jul 29 16:36:03 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 8F2C8BA78A7 for ; Fri, 29 Jul 2016 16:36:03 +0000 (UTC) (envelope-from ed@nuxi.nl) Received: from mailman.ysv.freebsd.org (unknown [127.0.1.3]) by mx1.freebsd.org (Postfix) with ESMTP id 727151AF4 for ; Fri, 29 Jul 2016 16:36:03 +0000 (UTC) (envelope-from ed@nuxi.nl) Received: by mailman.ysv.freebsd.org (Postfix) id 71C16BA78A6; Fri, 29 Jul 2016 16:36:03 +0000 (UTC) Delivered-To: arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 7165FBA78A4 for ; Fri, 29 Jul 2016 16:36:03 +0000 (UTC) (envelope-from ed@nuxi.nl) Received: from mail-yw0-x22c.google.com (mail-yw0-x22c.google.com [IPv6:2607:f8b0:4002:c05::22c]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 3DB991AF2 for ; Fri, 29 Jul 2016 16:36:03 +0000 (UTC) (envelope-from ed@nuxi.nl) Received: by mail-yw0-x22c.google.com with SMTP id u134so119867920ywg.3 for ; Fri, 29 Jul 2016 09:36:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuxi-nl.20150623.gappssmtp.com; s=20150623; h=mime-version:from:date:message-id:subject:to:cc; bh=yV/s/5PQr301UFbMgB5cINRP6hz4wsFIaICjqSExrqY=; b=OJEQ+9jx5y0iudHfeFjIJ9+XEXVhLYkkjNqrRuJjsTO5JAHpFMFwXlsE/Fy0rR0Wi3 tHjY5G4unSj6TRYk7EjSaL6N7FI9WnUt3X7rj/Vsm2oOdTcKXxQmCrNbIqnDFEfbOvVI I+lBVKxTXpHojE+HG4qtze+boeR29KejRAEhOHhYfSbTHDDtKMA8X2zzZnL+EqUM4Hyc TU1ifwz5nBV3rS+AajfRpg/mFx2+mNHL7p4LdicgZwJAcpuVgf+h/9jncuRk3KjI+6JV Lz4To3n+czDY9R27ilrz/A/edL0lOeFp5hdL0bxAnoSl1cBzErlXgfp5BS4H/WtVagwx hWvw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:from:date:message-id:subject:to:cc; bh=yV/s/5PQr301UFbMgB5cINRP6hz4wsFIaICjqSExrqY=; b=KyHOpEDQ1ncbAm3P4gYMBXkitqahuiAy4+uQsLf7cRmvouk7rlfxf1pWLfbP6RBRiJ VhPxpfnFlvuH9QsCUp9gPheuQLWYmY1r0Rf99bWxQHPEPypUCh+ZKSt1D2YUQsPSGDFn aYwu1dARVGUTPfzs5u1hp28NC5tMh2dgg/LkKR2XAdlrWC5gRpqjnj1HUPcycPC0+E56 IsY6/yV/at8wkyJTrs+Y2533kiVWceK7tSMyOo+o6IVrYF2Ed2OIopx6lGpw6Xusf8GM 7Rldn9DgC/i/VIyI3cddFHgZe0eWFYlTm1cWCA3Epnc743QirqlrfT6JWFum87AGNPXN 3LOg== X-Gm-Message-State: AEkooutqkguTEg97YUsCCdm2f4kOo+lgpjl20Rhk6PoOn0xSJOvefkn6vc9YLsQzSiWpYk2seqOHBxEiCi64XQ== X-Received: by 10.129.147.130 with SMTP id k124mr21506839ywg.116.1469810162070; Fri, 29 Jul 2016 09:36:02 -0700 (PDT) MIME-Version: 1.0 Received: by 10.13.201.71 with HTTP; Fri, 29 Jul 2016 09:36:01 -0700 (PDT) From: Ed Schouten Date: Fri, 29 Jul 2016 18:36:01 +0200 Message-ID: Subject: lib/libc/Versions.def: new symbol version for 12.x To: arch@freebsd.org Cc: Ed Maste Content-Type: text/plain; charset=UTF-8 X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Jul 2016 16:36:03 -0000 Hi there, In a couple of weeks from now I'm going to add new versions of basename() and dirname() to libc that are thread-safe. These should now use symbol version "FBSD_1.5", but a question that emaste@ and I have is: why don't these symbol version names follow the major versions of FreeBSD? As in, why not call this "FBSD_12.0" instead? Thoughts? -- Ed Schouten Nuxi, 's-Hertogenbosch, the Netherlands KvK-nr.: 62051717 From owner-freebsd-arch@freebsd.org Fri Jul 29 19:01:18 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 87D6ABA830F for ; Fri, 29 Jul 2016 19:01:18 +0000 (UTC) (envelope-from deischen@freebsd.org) Received: from mailman.ysv.freebsd.org (unknown [127.0.1.3]) by mx1.freebsd.org (Postfix) with ESMTP id 73DA91301 for ; Fri, 29 Jul 2016 19:01:18 +0000 (UTC) (envelope-from deischen@freebsd.org) Received: by mailman.ysv.freebsd.org (Postfix) id 6F93ABA830E; Fri, 29 Jul 2016 19:01:18 +0000 (UTC) Delivered-To: arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 6F36ABA830D for ; Fri, 29 Jul 2016 19:01:18 +0000 (UTC) (envelope-from deischen@freebsd.org) Received: from mail.netplex.net (mail.netplex.net [204.213.176.9]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "*.netplex.net", Issuer "RapidSSL SHA256 CA - G3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 377C71300; Fri, 29 Jul 2016 19:01:17 +0000 (UTC) (envelope-from deischen@freebsd.org) Received: from sea.ntplx.net (sea.ntplx.net [204.213.176.11]) by mail.netplex.net (8.15.1/8.15.1/NETPLEX) with ESMTP id u6TIra3P042400; Fri, 29 Jul 2016 14:53:36 -0400 X-Virus-Scanned: by AMaViS and Clam AntiVirus (mail.netplex.net) X-Greylist: Message whitelisted by DRAC access database, not delayed by milter-greylist-4.4.3 (mail.netplex.net [204.213.176.9]); Fri, 29 Jul 2016 14:53:36 -0400 (EDT) Date: Fri, 29 Jul 2016 14:53:36 -0400 (EDT) From: Daniel Eischen X-X-Sender: eischen@sea.ntplx.net Reply-To: Daniel Eischen To: Ed Schouten cc: arch@freebsd.org, Ed Maste Subject: Re: lib/libc/Versions.def: new symbol version for 12.x In-Reply-To: Message-ID: References: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Jul 2016 19:01:18 -0000 On Fri, 29 Jul 2016, Ed Schouten wrote: > Hi there, > > In a couple of weeks from now I'm going to add new versions of > basename() and dirname() to libc that are thread-safe. These should > now use symbol version "FBSD_1.5", but a question that emaste@ and I > have is: why don't these symbol version names follow the major > versions of FreeBSD? As in, why not call this "FBSD_12.0" instead? New symbol versions can be rolled back to -stable or other prior baselines. Also, if there are no changes in a major release branch, there's no need to bump the symbol version. It's just an arbitrary number somewhat tied to major release versions by convention. I started it at 1.0 when I first added symbol versioning, so you can probably blame me ;-) -- DE From owner-freebsd-arch@freebsd.org Fri Jul 29 19:11:08 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id AF9ECBA85A9 for ; Fri, 29 Jul 2016 19:11:08 +0000 (UTC) (envelope-from carpeddiem@gmail.com) Received: from mailman.ysv.freebsd.org (mailman.ysv.freebsd.org [IPv6:2001:1900:2254:206a::50:5]) by mx1.freebsd.org (Postfix) with ESMTP id 8F27F1A7C for ; Fri, 29 Jul 2016 19:11:08 +0000 (UTC) (envelope-from carpeddiem@gmail.com) Received: by mailman.ysv.freebsd.org (Postfix) id 8B17ABA85A8; Fri, 29 Jul 2016 19:11:08 +0000 (UTC) Delivered-To: arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 8AC07BA85A7 for ; Fri, 29 Jul 2016 19:11:08 +0000 (UTC) (envelope-from carpeddiem@gmail.com) Received: from mail-it0-x231.google.com (mail-it0-x231.google.com [IPv6:2607:f8b0:4001:c0b::231]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 534261A78; Fri, 29 Jul 2016 19:11:08 +0000 (UTC) (envelope-from carpeddiem@gmail.com) Received: by mail-it0-x231.google.com with SMTP id u186so200847164ita.0; Fri, 29 Jul 2016 12:11:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=HlcyQdQlF5FYofLSJyt0X8cs+K6wQp5ygoFV6SVTCh4=; b=k/mLQj5Mf9PTmYazJk4xE3jASV8Dt1MWi98ZWp62L0JdmtSmLZL7U0vEMuL23i12kB 1nfRh6rXaU46Rp17o+XvDbum2ZKUa8tiJ9W8jfW+C1mV5iUmHBcMo+9GIhB6mdus7WIH NXmxzh/+eyoahka7E8rnOIY6BhBwm9Pq2iAp7gRrFNd/QaqmiJtHUGAJUXf1r61D7RPB +QJ/XrE19oM2WEwpz/J5xDhIzjLOublo9A5N3nQNniJNLLNbBk2st8I+TlEerosFYGNd N/XZhpU2bZ8JERYXZMxLa5zvvNUf5077gCC5qso5flSg5gB0JUw3qMjfyP7FqQNL+nSu UCTA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=HlcyQdQlF5FYofLSJyt0X8cs+K6wQp5ygoFV6SVTCh4=; b=aygp1KG1gkKUXyU4+hGY/QyyX/l31wM0BHIAlk10ZA7/K0pWrLLS32ge7ySBq6gWH5 3ECOjXMKt0njwI5DamWrjK5Z5r8aYll+n8GFa9N5MwdKivMwHSRllvP34aEFJlgMstuA hbSMvG6X/igsITN4cckP1cCMX2Lk8eSmQ/MF76BKFa3R9As2yhrDm5vyH2D1Atff7ZDS syKVSRBHOIFaWv8wJEILOP+KDfe+ebO6p7ULAlaOt+syH6Eg3T14NyMY6pxCDR6GatrK uW6G23KR5FkIyy47qvxOlsPj4h5l2dSQiTPwZOKrTr/exL4Vb126X6/ghWCdh94p/LWg vyMA== X-Gm-Message-State: AEkooutVMiQG4Wynn1GSyhn45ZcBZ8W8/IBL9Tvp+0MVN8ipuoMgoJxbK2TDsc/ltf82NOjBpjq7jlOWingpQQ== X-Received: by 10.36.86.134 with SMTP id o128mr49493469itb.5.1469819467103; Fri, 29 Jul 2016 12:11:07 -0700 (PDT) MIME-Version: 1.0 Sender: carpeddiem@gmail.com Received: by 10.107.138.28 with HTTP; Fri, 29 Jul 2016 12:10:47 -0700 (PDT) In-Reply-To: References: From: Ed Maste Date: Fri, 29 Jul 2016 15:10:47 -0400 X-Google-Sender-Auth: l87ai8dFysIM16n44uKmWil8kKA Message-ID: Subject: Re: lib/libc/Versions.def: new symbol version for 12.x To: Daniel Eischen Cc: Ed Schouten , "freebsd-arch@freebsd.org" Content-Type: text/plain; charset=UTF-8 X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Jul 2016 19:11:08 -0000 On 29 July 2016 at 14:53, Daniel Eischen wrote: > > New symbol versions can be rolled back to -stable or other prior > baselines. If that happens I think having an FBSD_12.0 symbol in stable/11 actually makes it more clear what's happening, than if a FBSD_1.5 symbol appears in stable/11. > Also, if there are no changes in a major release branch, > there's no need to bump the symbol version. That's fine too, the version would just jump from FBSD_10.0 to FBSD_12.0 for example. From owner-freebsd-arch@freebsd.org Fri Jul 29 19:18:09 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id D7E30BA8849 for ; Fri, 29 Jul 2016 19:18:09 +0000 (UTC) (envelope-from ed@nuxi.nl) Received: from mailman.ysv.freebsd.org (mailman.ysv.freebsd.org [IPv6:2001:1900:2254:206a::50:5]) by mx1.freebsd.org (Postfix) with ESMTP id B74051D61 for ; Fri, 29 Jul 2016 19:18:09 +0000 (UTC) (envelope-from ed@nuxi.nl) Received: by mailman.ysv.freebsd.org (Postfix) id B361DBA8848; Fri, 29 Jul 2016 19:18:09 +0000 (UTC) Delivered-To: arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id B0C3ABA8846 for ; Fri, 29 Jul 2016 19:18:09 +0000 (UTC) (envelope-from ed@nuxi.nl) Received: from mail-yw0-x234.google.com (mail-yw0-x234.google.com [IPv6:2607:f8b0:4002:c05::234]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 692D41D5D for ; Fri, 29 Jul 2016 19:18:09 +0000 (UTC) (envelope-from ed@nuxi.nl) Received: by mail-yw0-x234.google.com with SMTP id u134so123642902ywg.3 for ; Fri, 29 Jul 2016 12:18:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuxi-nl.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=OBlCoAQ7nhF4i8nJIBxBen/TdQtnnRj98c0QcjnFdvQ=; b=zkyq+Rg0xHE6J7Jd0yWewlvi5dUgPKh07UdSbnVSMq/ZWE+BvQFurX+aUOyY5Gh+jw vKh8mUhdVqpZ25kvVtNHSYuIIPM0Jv89TBId7Cfy0K2MJIE46ASPxpa6CoNJOsdvojIO npPD8yTeu8JxSo12b1Y3plNToFdQudB0pkf0tpNQ6kzbJjyj3mtuwNa9j2CL0vZYABqf 7mjvEMaKBMfGwF4BqXUIhxN0R8zCWoiQDExkalPUu2/scXiPFo9CpYmL4xwbdwY88wRW CsF9eFy/ZjHS20DF1GYAzPMlpj1mIHEInkkJH5jgCI72Z6fqlT98weodW43OH7nREhc5 p8Ow== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=OBlCoAQ7nhF4i8nJIBxBen/TdQtnnRj98c0QcjnFdvQ=; b=Lgbalq/iIQlh1+84nweHErd9fC/BT/h+6lDQGJzI1rJondN2Xwz5pcOtdEu8GmNd9d xCGgkQh5BhuKaVkRM0d7DnzR3iaSGT9PnUC4XzN+7Xv+thgRKXJAOj6/KSI8qVfqTdIw gpUO9Ks4AN56ercKQ+c4cFLB6IdyW5kNlbLPhKA6fMeLA9c7A16yadaCRuqmMnGaKNrw ua7pWVRQk/R1mk14fDlz/lDCc9ay6/S2YxdnpsoaaC2/B7nebKU7R8+Nm37If116KqOG ZuhqiyR1JXSNOB/CIA+SFOJ+uak112eMywotffpzKtzN1fvK4oeH7iNieEorLaqBzzVL tXlw== X-Gm-Message-State: AEkoouuNREIOi9O8U9dMJG7P/5YOHtDKj0PJUD0m9lCmePOGMmeLWE06JcJh35/bzrxzA6YYcad2z7w0LZCRDw== X-Received: by 10.129.98.2 with SMTP id w2mr14276107ywb.313.1469819888438; Fri, 29 Jul 2016 12:18:08 -0700 (PDT) MIME-Version: 1.0 Received: by 10.13.201.71 with HTTP; Fri, 29 Jul 2016 12:18:08 -0700 (PDT) In-Reply-To: References: From: Ed Schouten Date: Fri, 29 Jul 2016 21:18:08 +0200 Message-ID: Subject: Re: lib/libc/Versions.def: new symbol version for 12.x To: Ed Maste Cc: Daniel Eischen , "freebsd-arch@freebsd.org" Content-Type: text/plain; charset=UTF-8 X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Jul 2016 19:18:09 -0000 2016-07-29 21:10 GMT+02:00 Ed Maste : > On 29 July 2016 at 14:53, Daniel Eischen wrote: >> >> New symbol versions can be rolled back to -stable or other prior >> baselines. > > If that happens I think having an FBSD_12.0 symbol in stable/11 > actually makes it more clear what's happening, than if a FBSD_1.5 > symbol appears in stable/11. Yes, exactly. The version number would just indicate the version of -CURRENT that was used to introduce the symbol. If we would be going down this road, then I have to further questions: - Should we drop the .0 suffix then? - Would it make sense to also name it to 'FreeBSD' instead of 'fBSD'? That is, using 'FreeBSD_12' as the next symbol version. -- Ed Schouten Nuxi, 's-Hertogenbosch, the Netherlands KvK-nr.: 62051717 From owner-freebsd-arch@freebsd.org Fri Jul 29 21:37:03 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 650BCBA8227 for ; Fri, 29 Jul 2016 21:37:03 +0000 (UTC) (envelope-from deischen@freebsd.org) Received: from mailman.ysv.freebsd.org (mailman.ysv.freebsd.org [IPv6:2001:1900:2254:206a::50:5]) by mx1.freebsd.org (Postfix) with ESMTP id 4E2B21337 for ; Fri, 29 Jul 2016 21:37:03 +0000 (UTC) (envelope-from deischen@freebsd.org) Received: by mailman.ysv.freebsd.org (Postfix) id 4D82FBA8226; Fri, 29 Jul 2016 21:37:03 +0000 (UTC) Delivered-To: arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 4D226BA8225 for ; Fri, 29 Jul 2016 21:37:03 +0000 (UTC) (envelope-from deischen@freebsd.org) Received: from mail.netplex.net (mail.netplex.net [204.213.176.9]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "*.netplex.net", Issuer "RapidSSL SHA256 CA - G3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 137471336; Fri, 29 Jul 2016 21:37:02 +0000 (UTC) (envelope-from deischen@freebsd.org) Received: from [10.0.0.51] (ip-414b102e.ct.fixed.ntplx.com [65.75.16.46]) (authenticated bits=0) by mail.netplex.net (8.15.1/8.15.1/NETPLEX) with ESMTPSA id u6TLb0Ts026448 (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NOT); Fri, 29 Jul 2016 17:37:00 -0400 X-Virus-Scanned: by AMaViS and Clam AntiVirus (mail.netplex.net) X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.4.3 (mail.netplex.net [204.213.176.9]); Fri, 29 Jul 2016 17:37:01 -0400 (EDT) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (1.0) Subject: Re: lib/libc/Versions.def: new symbol version for 12.x From: Daniel Eischen X-Mailer: iPhone Mail (13G34) In-Reply-To: Date: Fri, 29 Jul 2016 17:37:00 -0400 Cc: Ed Maste , "freebsd-arch@freebsd.org" Content-Transfer-Encoding: quoted-printable Message-Id: <4647100A-67D9-416D-8E34-4CC1F450227A@freebsd.org> References: To: Ed Schouten X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Jul 2016 21:37:03 -0000 Sent from my iPhone > On Jul 29, 2016, at 3:18 PM, Ed Schouten wrote: >=20 > 2016-07-29 21:10 GMT+02:00 Ed Maste : >>> On 29 July 2016 at 14:53, Daniel Eischen wrote: >>>=20 >>> New symbol versions can be rolled back to -stable or other prior >>> baselines. >>=20 >> If that happens I think having an FBSD_12.0 symbol in stable/11 >> actually makes it more clear what's happening, than if a FBSD_1.5 >> symbol appears in stable/11. >=20 > Yes, exactly. The version number would just indicate the version of > -CURRENT that was used to introduce the symbol. If we would be going > down this road, then I have to further questions: >=20 > - Should we drop the .0 suffix then? > - Would it make sense to also name it to 'FreeBSD' instead of 'fBSD'? >=20 > That is, using 'FreeBSD_12' as the next symbol version. They were originally modeled similar to Solaris (and Linux to some extent, I= IRC): https://docs.oracle.com/cd/E19683-01/806-4125/solarisabi-8/index.html I think there's not much point of changing the naming scheme now, it's not l= ike it's visible to the typical user. The comment in the Version.defs file p= retty much explains everything. --=20 DE= From owner-freebsd-arch@freebsd.org Fri Jul 29 23:16:27 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 332D8BA75D6; Fri, 29 Jul 2016 23:16:27 +0000 (UTC) (envelope-from onwahe@gmail.com) Received: from mail-io0-f179.google.com (mail-io0-f179.google.com [209.85.223.179]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id F24F211EA; Fri, 29 Jul 2016 23:16:26 +0000 (UTC) (envelope-from onwahe@gmail.com) Received: by mail-io0-f179.google.com with SMTP id q83so142358604iod.1; Fri, 29 Jul 2016 16:16:26 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=1/dzjY4rjQhu54eGdT3PiIKE5UVeyEAndD2vRgbSnzI=; b=aETbDCHCz4ywEilPcMPYCXkMwL9Tc6Wu0e8QeMDoxL35I4iznmFyihDvLmMcgvgneZ +Ew3snOLOyLq+xRagq1B2zHDCwJxWEspsrAVfCE/lSmt0WxExa/wOcySCvUYphY7f7UL /gq6chGap5kZscmOdZ6qhQTODfCmhU0sz/XgEn5AW+vsJoemjekoNIFB6tObUfohWLvQ LrH/q5aIMyZ6au6II3cY5BuClKEAFP12I8LqF5ZYLXVKAesewUC1e6gUSDKtKn6XKVh3 v26SSuxa+DYpw5GhJzEYsigiKxEM7ET4Ugv0yAPqukoNzp3ogKP5jrTi3gNJ+M7g6dPb g8GQ== X-Gm-Message-State: AEkoouv4rjU94FXcr2P2NwpXt0xA9ghnpZffjVLy7F6qeygb5vv6Mequ0oeqb4WpP+v7Hw== X-Received: by 10.107.175.27 with SMTP id y27mr52111077ioe.137.1469830590332; Fri, 29 Jul 2016 15:16:30 -0700 (PDT) Received: from mail-io0-f177.google.com (mail-io0-f177.google.com. [209.85.223.177]) by smtp.gmail.com with ESMTPSA id e201sm2152444itc.21.2016.07.29.15.16.30 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 29 Jul 2016 15:16:30 -0700 (PDT) Received: by mail-io0-f177.google.com with SMTP id b62so140912024iod.3; Fri, 29 Jul 2016 15:16:30 -0700 (PDT) X-Received: by 10.107.142.129 with SMTP id q123mr38715103iod.84.1469830589842; Fri, 29 Jul 2016 15:16:29 -0700 (PDT) MIME-Version: 1.0 Received: by 10.64.162.75 with HTTP; Fri, 29 Jul 2016 15:16:28 -0700 (PDT) In-Reply-To: References: <201606051620.u55GKD5S066398@repo.freebsd.org> <57907B0F.9070204@FreeBSD.org> <9d2a224c-b787-2875-5984-a7a2354e8695@freebsd.org> <57934ABD.6010807@FreeBSD.org> <4e7a3e8f-cc21-f5f2-e3e0-4dbd554a4cd0@freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <57961549.4020105@FreeBSD.org> <57976867.6080705@FreeBSD.org> <5798E104.5020104@FreeBSD.org> <579A25BB.8070206@FreeBSD.org> <30790e40-58b4-3371-c0f0-b7545571f389@freebsd.org> <579AFFC5.1040005@FreeBSD.org> From: Svatopluk Kraus Date: Sat, 30 Jul 2016 00:16:28 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: INTRNG (Was: svn commit: r301453....) To: Nathan Whitehorn Cc: Michal Meloun , "freebsd-arm@freebsd.org" , "freebsd-arch@freebsd.org" Content-Type: text/plain; charset=UTF-8 X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Jul 2016 23:16:27 -0000 Well, I'm online again, unfortanately, I'm quite busy, so just quick response for now to the formal ask for the reversion. On Fri, Jul 29, 2016 at 4:41 PM, Nathan Whitehorn wrote: > [snip] > > So here is where I am right now: > - The perceived advantage of the new API seems to be that the mapping > information (interrupt parent, etc.) ends up in a struct resource instead of > in a centralized mapping table It's more than that. The change has made INTRNG framework not dependent on OFW(FDT) stuff. So after next r301543, the framework is clean of all additions related to various buses. It was something I wanted to do before FreeBSD-11 release, once when adrian@ moved INTRNG to sys/kern. The framework is used by arm, arm64 and mips now, so from my point of view, it was quite important. The way how it was done is not perfect and may be changed in the future. However, it does not break anything, does not change old functionality, and only few bus drivers were modified slightly. It was also only way which I and Michal have agreed on considering current kernel code. > - Additionally, it gives you a better shot at having the PIC online before > the PIC's interrupts are parsed (which is not required, but nice). For me, it's just correct design. Please, not all world is about OFW. > - Beyond these aesthetic points, there are no concrete examples of new > functionality added by this API, aside from some minor implementation bugs > of the old one on ARM that are easily fixed. Please, don't use subjective attributes like "aesthetic". > - There are, conversely, a number of concrete cases where this new API would > not be able to do the right thing. Some of these can be worked around or > fixed with significant restructuring in the MI parts of the kernel. I have not looked yet at these concrete cases, but which MI parts of kernel do you think of? It may be supposed that some bus drivers would be modified, but not MI parts of kernel! > - If we have both, the interrupt handling code in the MI parts of the kernel > will bifurcate. This patch alone has added a bunch of #ifdef to > kern/subr_bus.c, a new file to kern/, and lots of #ifdef to dev/ofw. This is > going to be really hard to maintain if we need both. IMO, this point is bogus totally. The #ifdefs in kern/subr_bus.c were added just to be polite to not-INTRNG kernels. They can be removed. No one new file was added. Lots of #ifdef in dev/ofw were added because ofw_bus_map_intr() return value is not checked in ofw_bus_intr_to_rl(), so resource list entry is always added. They are there also to clearly manifest INTRNG needs. > > Is this all correct? > I don't think so. Further, considering the reversion, I don't think that it can be done simply now. I suppose that more files were changed afterwards when no bus header is polluted by the framework now. However, as I have already wrote, this part of INTRNG may be changed to serve well for everyone. On the other hand, IMO, a centralized global interrupt mapping table is not good design, and if established after all, it should not be a part of the framework. That said, I suggest to continue with work on INTRNG. Svata > If so, can we please back this out until this discussion is complete? I'm > asking this formally at this point, under the Committer's Guide section > about reversion requests. > -Nathan > > From owner-freebsd-arch@freebsd.org Sat Jul 30 01:35:40 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id DDAFABA7F92; Sat, 30 Jul 2016 01:35:40 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from c.mail.sonic.net (c.mail.sonic.net [64.142.111.80]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id BBAF412E5; Sat, 30 Jul 2016 01:35:40 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from zeppelin.tachypleus.net (75-101-50-44.static.sonic.net [75.101.50.44]) (authenticated bits=0) by c.mail.sonic.net (8.15.1/8.15.1) with ESMTPSA id u6U1ZUpP029127 (version=TLSv1.2 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Fri, 29 Jul 2016 18:35:31 -0700 Subject: Re: INTRNG (Was: svn commit: r301453....) To: Svatopluk Kraus References: <201606051620.u55GKD5S066398@repo.freebsd.org> <4e7a3e8f-cc21-f5f2-e3e0-4dbd554a4cd0@freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <57961549.4020105@FreeBSD.org> <57976867.6080705@FreeBSD.org> <5798E104.5020104@FreeBSD.org> <579A25BB.8070206@FreeBSD.org> <30790e40-58b4-3371-c0f0-b7545571f389@freebsd.org> <579AFFC5.1040005@FreeBSD.org> Cc: Michal Meloun , "freebsd-arm@freebsd.org" , "freebsd-arch@freebsd.org" From: Nathan Whitehorn Message-ID: <8efe4828-ab2a-02a6-902b-8614b1f4b24e@freebsd.org> Date: Fri, 29 Jul 2016 18:35:30 -0700 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:45.0) Gecko/20100101 Thunderbird/45.1.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Sonic-CAuth: UmFuZG9tSVaAqmOkWSG1yDcFr4npweYNEYuVMAL+kwkFbqCX8t7eFnj92NW0YBGQFI6dUiiaLP7nVCu76DwRvecJujjdvHtxAJTIl3ekOaE= X-Sonic-ID: C;6gmc5vVV5hGfPJtMTlz00w== M;1Kz65vVV5hGfPJtMTlz00w== X-Spam-Flag: No X-Sonic-Spam-Details: 0.0/5.0 by cerberusd X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 30 Jul 2016 01:35:41 -0000 On 07/29/16 15:16, Svatopluk Kraus wrote: > Well, I'm online again, unfortanately, I'm quite busy, so just quick > response for now to the formal ask for the reversion. > > On Fri, Jul 29, 2016 at 4:41 PM, Nathan Whitehorn > wrote: > [snip] > >> So here is where I am right now: >> - The perceived advantage of the new API seems to be that the mapping >> information (interrupt parent, etc.) ends up in a struct resource instead of >> in a centralized mapping table > It's more than that. The change has made INTRNG framework not > dependent on OFW(FDT) stuff. So after next r301543, the framework is > clean of all additions related to various buses. It was something I > wanted to do before FreeBSD-11 release, once when adrian@ moved INTRNG > to sys/kern. The framework is used by arm, arm64 and mips now, so from > my point of view, it was quite important. The way how it was done is > not perfect and may be changed in the future. However, it does not > break anything, does not change old functionality, and only few bus > drivers were modified slightly. It was also only way which I and > Michal have agreed on considering current kernel code. Except that the API cannot support all the existing things, so now we have two APIs to support for the lifetime of the 11.x branch. This needed extensive review from platform maintainers and from arch@, which it did not get. > >> - Additionally, it gives you a better shot at having the PIC online before >> the PIC's interrupts are parsed (which is not required, but nice). > For me, it's just correct design. Please, not all world is about OFW. Of course not! But it does affect OFW platforms, which worked fine before. I also disagree strongly about the "correct design" in this case. This API is fragile since it requires coordination between resource list enumeration and allocation and it provides fewer features than the old one (which was not OFW-only either). >> - Beyond these aesthetic points, there are no concrete examples of new >> functionality added by this API, aside from some minor implementation bugs >> of the old one on ARM that are easily fixed. > Please, don't use subjective attributes like "aesthetic". If the issues are not aesthetic, then what concrete, technical problems does this solve? > > >> - There are, conversely, a number of concrete cases where this new API would >> not be able to do the right thing. Some of these can be worked around or >> fixed with significant restructuring in the MI parts of the kernel. > I have not looked yet at these concrete cases, but which MI parts of > kernel do you think of? It may be supposed that some bus drivers would > be modified, but not MI parts of kernel! If you don't want a global IRQ lookup table, the PCI interrupt routing APIs need to change, for one thing. You would also need a partial delayed attach mechanism to handle bus bridges that themselves have interrupts (hot-plug bridges, for example) and might have interrupt controllers as children athat does not currently exist in newbus. > >> - If we have both, the interrupt handling code in the MI parts of the kernel >> will bifurcate. This patch alone has added a bunch of #ifdef to >> kern/subr_bus.c, a new file to kern/, and lots of #ifdef to dev/ofw. This is >> going to be really hard to maintain if we need both. > IMO, this point is bogus totally. The #ifdefs in kern/subr_bus.c were > added just to be polite to not-INTRNG kernels. They can be removed. No > one new file was added. Lots of #ifdef in dev/ofw were added because > ofw_bus_map_intr() return value is not checked in > ofw_bus_intr_to_rl(), so resource list entry is always added. They are > there also to clearly manifest INTRNG needs. #ifdef or not, there are now two unrelated mechanisms and code paths for dealing with device tree interrupts. One of them cannot possibly be used on PowerPC (this one) and so there will have to be two parallel code paths in perpetuity. >> Is this all correct? >> > I don't think so. Further, considering the reversion, I don't think > that it can be done simply now. I suppose that more files were changed > afterwards when no bus header is polluted by the framework now. > However, as I have already wrote, this part of INTRNG may be changed > to serve well for everyone. On the other hand, IMO, a centralized > global interrupt mapping table is not good design, and if established > after all, it should not be a part of the framework. That said, I > suggest to continue with work on INTRNG. It cannot be done simply because the code was checked in without review or warning during a code slush with a cryptic commit message and now we are stuck with it for 11.x. In HEAD, there are only a few commits based on this that seem to provide no functional changes, so I really don't think reverting would be that bad. My concerns here are technical and I will stop complaining about this instantly if: 1. Anyone can point me to a single concrete example of a problem solved by this API that could not be solved with the existing code. You spent time writing this code, so there must be such a case! 2. Anyone can tell me how to implement the cases in my email to arch and on the wiki at https://wiki.freebsd.org/Complicated_Interrupts using this API. These work perfectly fine with the normal newbus APIs but appear to break with this one. This affects me directly since I maintain, or try to maintain, the content of dev/ofw and a platform (PowerPC) that relies on this code. I'm happy to switch the interrupt architecture, but it needs to be at least as functional as the old code to do that. Ideally, it would also be *more* functional so that it wasn't just churn -- but I would be willing to do the work regardless. As it is, however, having this new API that seemingly can't be supported on one of our platforms imposes a significant burden on doing those things. That absolutely needs to be resolved before we can continue here. The normal procedure would be to revert, have a discussion, and then recommit. If you refuse to do that, or to have any meaningful discussion about the design of this patch, I am not sure what the path forward is. -Nathan > > Svata > > >> If so, can we please back this out until this discussion is complete? I'm >> asking this formally at this point, under the Committer's Guide section >> about reversion requests. >> -Nathan >> >> From owner-freebsd-arch@freebsd.org Sat Jul 30 06:55:11 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 0FC30BA9774; Sat, 30 Jul 2016 06:55:11 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: from mail-it0-x235.google.com (mail-it0-x235.google.com [IPv6:2607:f8b0:4001:c0b::235]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id C4CB21AFB; Sat, 30 Jul 2016 06:55:10 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: by mail-it0-x235.google.com with SMTP id f6so208922225ith.1; Fri, 29 Jul 2016 23:55:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=r3zXziOYd9pqy0GFrs1nO7oSeiuRS/Krdz943pVwbRo=; b=KqrV5gxuRWwARwUMLZiz3kLcaCSk30LU5dzZm4wo2Uqoie2e5cX5nrtZO4lpJ+piBh q3fLaheXESvmtWfKDb8/vgIYfe/8Idr+iOxOvMhhDKPpAy0VjCew6YbYs0o/datpam8r zqg57BvQL0QYOJoiWuvO6wOzSz1y2Nvgq0juQNsFcoK+QZNIcibpbWd8m85qiKlU77nD 7HYQy6Ouq12HpGUZPvCj+Lc676mXo5XnteteYciqK2TWG2gIe6f/gjr+drcDm7hhmXnB PxlqMS1DL6h1xBYlbXMkHWVU/yYg62ePXIoB41IUyliUut/vVubagPS8cvC6oVIT/dfb dd1g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=r3zXziOYd9pqy0GFrs1nO7oSeiuRS/Krdz943pVwbRo=; b=PXHBzFaBffAZiIcz5AMLVWSDnzI1fcm+cMXw9itKb4G1ltThoE5/1GLWjBUJquOJFM B1zxsQTAmLV/dgcaY0EOR/e+k6TcOjW24pPFtCkPwkViYf8mU8/FpfXLfNoQlLQ77fUK ikL7Qmafl/axsPEQWbdKWfnecFJuD04vuZREvNFmhlcUgku7kb1PHh5B+MsspSKfLYu0 NNnuRJ//8V1nKd/GbIpQkfudUt2wjYCABtd4tBuaGxMnfPaBrv25DgS6pLYwHMQY8/6F owFkipcSRRc9kSAzJmim3WuRPreq0BLPwo1Z7dgZYVM33pTApYiXw4zSMFKK0mB8Dke/ BmHw== X-Gm-Message-State: AEkoouum/9iVtMhobtoNNuwJsrSzcwlVl3FwfUD5/oXU9wco+xedaBS20Bci33q8kaFZS2JVR9ZF+MvDjQMwNQ== X-Received: by 10.36.101.195 with SMTP id u186mr45792625itb.80.1469861709935; Fri, 29 Jul 2016 23:55:09 -0700 (PDT) MIME-Version: 1.0 Received: by 10.36.141.129 with HTTP; Fri, 29 Jul 2016 23:55:08 -0700 (PDT) In-Reply-To: References: <201606051620.u55GKD5S066398@repo.freebsd.org> <57907B0F.9070204@FreeBSD.org> <9d2a224c-b787-2875-5984-a7a2354e8695@freebsd.org> <57934ABD.6010807@FreeBSD.org> <4e7a3e8f-cc21-f5f2-e3e0-4dbd554a4cd0@freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <57961549.4020105@FreeBSD.org> <57976867.6080705@FreeBSD.org> <5798E104.5020104@FreeBSD.org> <579A25BB.8070206@FreeBSD.org> <30790e40-58b4-3371-c0f0-b7545571f389@freebsd.org> <579AFFC5.1040005@FreeBSD.org> From: Adrian Chadd Date: Fri, 29 Jul 2016 23:55:08 -0700 Message-ID: Subject: Re: INTRNG (Was: svn commit: r301453....) To: Svatopluk Kraus Cc: Nathan Whitehorn , "freebsd-arm@freebsd.org" , Michal Meloun , "freebsd-arch@freebsd.org" Content-Type: text/plain; charset=UTF-8 X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 30 Jul 2016 06:55:11 -0000 On 29 July 2016 at 15:16, Svatopluk Kraus wrote: > Well, I'm online again, unfortanately, I'm quite busy, so just quick > response for now to the formal ask for the reversion. > > On Fri, Jul 29, 2016 at 4:41 PM, Nathan Whitehorn > wrote: >> > > [snip] > >> >> So here is where I am right now: >> - The perceived advantage of the new API seems to be that the mapping >> information (interrupt parent, etc.) ends up in a struct resource instead of >> in a centralized mapping table > > It's more than that. The change has made INTRNG framework not > dependent on OFW(FDT) stuff. So after next r301543, the framework is > clean of all additions related to various buses. It was something I > wanted to do before FreeBSD-11 release, once when adrian@ moved INTRNG > to sys/kern. The framework is used by arm, arm64 and mips now, so from > my point of view, it was quite important. The way how it was done is > not perfect and may be changed in the future. However, it does not > break anything, does not change old functionality, and only few bus > drivers were modified slightly. It was also only way which I and > Michal have agreed on considering current kernel code. > > >> - Additionally, it gives you a better shot at having the PIC online before >> the PIC's interrupts are parsed (which is not required, but nice). > > For me, it's just correct design. Please, not all world is about OFW. Hi, Right, but the world does include open firmware, and Nathan/Justin have a lot of experience over on the powerpc side with all the crazy variants on interrupt enumeration and handling there. I think you and Nathan are sort of on the same page, but sort of not on the same page, and Nathan is trying to ensure things don't become more complicated moving forward. I really do suggest this stuff get written down and thought about a little more, and not just in the context of the ARM style GIC/PIC+GPIO interrupt setups that people are writing code for. Don't get me wrong, I'm glad it's happening, but there are more consumers involved than just ARM. :) So, are you/Michal willing to describe some of the ARM interrupt/platform hilarity on his wiki page (https://wiki.freebsd.org/Complicated_Interrupts ) so everyone can be on the same page (heh!) ? Right now there's a lot of information floating around in emails but it's hard to pull out exactly what's going on unless you're knee deep in it all working on that platform. I'm trying to figure it out too for the MIPS + GPIO interrupt stuff that people are now doing with the QCA MIPS hardware and I'd like to make sure I know what's required for the QCA ARM (Dakota/IPQ8019) hardware. Thanks! -adrian -adrian From owner-freebsd-arch@freebsd.org Sat Jul 30 07:09:15 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 187C4BA9AF8 for ; Sat, 30 Jul 2016 07:09:15 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mailman.ysv.freebsd.org (mailman.ysv.freebsd.org [IPv6:2001:1900:2254:206a::50:5]) by mx1.freebsd.org (Postfix) with ESMTP id F2B06181D for ; Sat, 30 Jul 2016 07:09:14 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: by mailman.ysv.freebsd.org (Postfix) id F2199BA9AF6; Sat, 30 Jul 2016 07:09:14 +0000 (UTC) Delivered-To: arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id F1C13BA9AF5 for ; Sat, 30 Jul 2016 07:09:14 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 9C6BE181C; Sat, 30 Jul 2016 07:09:14 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.15.2/8.15.2) with ESMTPS id u6U7985Y005777 (version=TLSv1 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Sat, 30 Jul 2016 10:09:08 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua u6U7985Y005777 Received: (from kostik@localhost) by tom.home (8.15.2/8.15.2/Submit) id u6U798WA005776; Sat, 30 Jul 2016 10:09:08 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Sat, 30 Jul 2016 10:09:08 +0300 From: Konstantin Belousov To: Daniel Eischen Cc: Ed Schouten , "freebsd-arch@freebsd.org" , Ed Maste Subject: Re: lib/libc/Versions.def: new symbol version for 12.x Message-ID: <20160730070908.GM83214@kib.kiev.ua> References: <4647100A-67D9-416D-8E34-4CC1F450227A@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4647100A-67D9-416D-8E34-4CC1F450227A@freebsd.org> User-Agent: Mutt/1.6.1 (2016-04-27) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.1 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on tom.home X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 30 Jul 2016 07:09:15 -0000 On Fri, Jul 29, 2016 at 05:37:00PM -0400, Daniel Eischen wrote: > They were originally modeled similar to Solaris (and Linux to some extent, IIRC): > > https://docs.oracle.com/cd/E19683-01/806-4125/solarisabi-8/index.html > > I think there's not much point of changing the naming scheme now, it's not like it's visible to the typical user. The comment in the Version.defs file pretty much explains everything. What we have currently in the symbol versioning scheme definitely has nothing common with Solaris, and has only slight resemblance to the glibc variant. Our rtld implements the GNU extention of the original Solaris versioning, but the way we provide actual versioning in shipped libraries defeats some useful parts of the functionality. The original Solaris scheme, AFAIR, has the goal of preventing binary linked to a newer library, even to try to start with the older version of the library. Solaris libc, when introducing a new symbol, also added a new version to the list of versions implemented by the library. On startup, dynamic linker verifies that all referenced versions are present in the library. This is needed since default binding resolution is lazy, and missed symbol goes undetected until called. As result, applications fail hard somewhere at runtime, surprising users and corrupting data, unless checked. We do exactly opposite, all symbols added during the CURRENT-X development cycle, are added into single namespace. This would be not too bad, we do not care much about CURRENT ABI until it is backward compatible, but we also merge symbols to stable branches under same namespace. The result is that you cannot distinguish older and newer libraries from the same stable branch at the startup linking. >From what I remember, both Solaris and glibc create new namespace for each symbols addition. GNU scheme has more flexibility by allowing to change ABI of symbols, and this is the only feature we use in limited form (we cannot provide ABI compat shims for symbols which change more than once during one CURRENT cycle). IMO it does not make sense to change current scheme without fixing the bug I described above. Any change in naming makes the things even more confusing, and imposing the pain only for somebody aestetic feel does not look right. From owner-freebsd-arch@freebsd.org Sat Jul 30 16:18:39 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id E1F45BA9F15; Sat, 30 Jul 2016 16:18:39 +0000 (UTC) (envelope-from mmel@FreeBSD.org) Received: from mail.miracle.cz (mail.miracle.cz [193.84.128.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mail.miracle.cz", Issuer "Miracle Group Root CA" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 986EB1C7F; Sat, 30 Jul 2016 16:18:39 +0000 (UTC) (envelope-from mmel@FreeBSD.org) Received: from [193.84.128.50] (meloun.ad.miracle.cz [193.84.128.50]) by mail.miracle.cz (Postfix) with ESMTPSA id 5239F3AC96; Sat, 30 Jul 2016 18:18:30 +0200 (CEST) Subject: Re: INTRNG (Was: svn commit: r301453....) To: Nathan Whitehorn References: <201606051620.u55GKD5S066398@repo.freebsd.org> <57934ABD.6010807@FreeBSD.org> <4e7a3e8f-cc21-f5f2-e3e0-4dbd554a4cd0@freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <57961549.4020105@FreeBSD.org> <57976867.6080705@FreeBSD.org> <5798E104.5020104@FreeBSD.org> <579A25BB.8070206@FreeBSD.org> <30790e40-58b4-3371-c0f0-b7545571f389@freebsd.org> <579AFFC5.1040005@FreeBSD.org> Cc: "freebsd-arm@freebsd.org" , Svatopluk Kraus , "freebsd-arch@freebsd.org" From: Michal Meloun X-Enigmail-Draft-Status: N1110 Message-ID: <579CD355.1050203@FreeBSD.org> Date: Sat, 30 Jul 2016 18:18:29 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.4.3 (mail.miracle.cz); Sat, 30 Jul 2016 18:18:30 +0200 (CEST) X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 30 Jul 2016 16:18:40 -0000 Dne 29.07.2016 v 16:41 Nathan Whitehorn napsal(a): [snip] Svata is online now, so i think that he is more competent for responding to all questions about INTRNG internals. >>>> But, as you wrote, INTRNG code is "absolutely free to map and >>>> dispatch >>>> multiple virtual IRQs from the same shared interrupt pin" (allow me to >>>> expand this also to INTRNG code itself) . >>>> In attempt to simply situation abound cache, in first step, we >>>> removed >>>> duplicates check from above model. This also removes unpleasantly >>>> sharing of entries. >>>> This step converts cache to some sort of duplicate of resource list >>>> entries. Each existing RLE have exactly one corresponding entry in >>>> cache. >>>> Also, the cache is not needed for INTRNG core. At this point, its >>>> only >>>> transfers data from ofw_bus_intr_to_rl() to consumers PIC. >>>> >>>> So, if we are able to attach 'key' to RLE, then we can remove the >>>> whole >>>> cache machinery from INTRNG. >>>> Do you agree? >>> No. Because you have to support two things that this still can't allow: >>> 1. Devices attaching before their interrupt controllers. >> You still expect that data readed from OFW must be parsed at RLE >> creation time. I don't see single reason why we cannot postpone >> parsing to bus_alloc_resource() time. >> At this point, its irrelevant if bus_alloc_resource() supports >> detached PIC or not. > > Because there are *lots* of cases in the kernel where interrupts are > represented as a number and not a struct resource *. I've listed many > of them; I'm sure there are more. PCI is the most notable example > (both LSI and MSI). For both of these, the route_interrupt (LSI) and > alloc_msi[x]() (MSI) functions give the bus driver a chance to look at > the device tree and then they return an IRQ number. There doesn't seem > to be any way to map that back to a struct resource * or something > besides an internal table. > > Moreover, there are cases (self-added interrupts by children, for > instance) in which the RL assigned by the parent does not reflect the > final RL used by the child and there is no way for the parent bus to > map an RID to a device tree entry. > > When we moved to doing this at interrupt assignment time rather than > resource allocation time or another occasion on PowerPC, it was for > these kinds of reasons. There were ways to do it later, but they were > hugely invasive and involved changing large parts of the kernel. Even > then, it would still be fragile: Trying to guess what device tree > entry was meant at bus_alloc_resource() time is much more error-prone > than doing the mapping when reading that device tree to assign the > resources in the first place, when you can make guarantees. > >> >>> 2. Interrupts encoded entirely in a 32-bit integer rather than a >>> struct resource * in a shared rman. This is required to support PCI >>> (both LSI and MSI). >>> >>> #2 could of course be fixed by completely retooling the API of the PCI >>> bus code, but, since it fixes something that currently is not a >>> problem, why would we do that? >> All relevant bus functions have struct resource * as an argument. And, >> in post r301453 kernel, all necessary data are attached to it. >> I don't see any reason for API change. > > There is: > int PCIB_ROUTE_INTERRUPT(device_t, device_t, pin) > > Which takes a PIN (A, B, C, D) and returns the IRQ to add to the RL. > This is the only real opportunity to parse an interrupt-map and does > not deal with struct resource. > > There are also PCIB_ALLOC_MSI[X], which take a device_t and count and > return a vector of IRQ numbers. > > How would you do resource allocation given these APIs? There seem to > be some dubious-looking attempts at generic MSI mapping code in the > new kern/subr_intr.c that don't allow the specification of more than > 32-bit interrupt specifiers and otherwise exactly duplicate the > pre-301453 flow for MSIs, but in a more complicated and > less-functional way. > > (While investigating this, I also just found > dev/pci/pci_host_generic.c, which mostly does the wrong things, is > actually specific to a couple of ARM boards, and duplicates code in > dev/ofw/ofwpci.c. But that's another discussion...) [snip][2] >>>>>> Each OFW device has a lot of dependencies. It must enable clocks, >>>>>> power >>>>>> regulators..., it must be taken from reset. All these action must be >>>>>> done before we can access single register. >>>>>> Most of these action can be done only with attached >>>>>> clock/power/reset >>>>>> controller. >>>>>> Within this, interrupts are not special. >>>>> They are actually quite special in that the kernel enables them late >>>>> and so you can defer setup. They are also special in that, for that >>>>> reason, it is possible to design systems with circular dependency >>>>> graphs with interrupts. It is not possible to do this with, for >>>>> example, clocks: if I need to apply a clock to the clock controller >>>>> with itself, the system is just not bootable and such a system will >>>>> not be built or shipped. Interrupts need only happen later, after >>>>> device setup, and so such systems *are* designed and shipped. The >>>>> same >>>>> is true for power. >>>> The G5 problem is standard 'cross type' circular dependency. You >>>> must >>>> (at BUS_PASS_BUS) initialize memory address window of PCI bridge and >>>> start bus enumeration. >>>> Rest of PCI bridge initialization (including all IRQ stuff) must be >>>> postponed to any later pass. Of course, all interrupt controllers must >>>> be attached at BUS_PASS_INTERRUPT. >>>> Or I missed something? >>> Yes. As I said, you could solve it that way. It would, of course, >>> require a massive retooling of newbus (to allow partial delayed >>> attach), everything in dev/pci (which doesn't expect that), and >>> (almost) every PCI driver in and out of the tree. >> I still think that only host to PCI bridge must me modified, and PCI-PCI >> bridge must be run at BUS_PASS_BUS (which is single line modification). > > On Powermacs, one PIC is 5 levels deep in the bus hierarchy, under 3 > PCI<->PCI bridges and a device that also has an ATA controller and a > bunch of other things. Yes, I see. And, as a said before, it's pretty common situation in OFW based world. Imho, all whats is needed is: https://github.com/strejda/freebsd/commit/96ab64e04dcebb131643c2ae1cf373194d23e8e5 (of course, it's not tested, I havn't access to powermac HW) It's hard for me to understand why you say that this change requires "massive retooling of newbus, everything in dev/pci, every PCI driver). > Some of those can have their own interrupts (which is fairly common > for error reporting or PCI-E hotplug). It wouldn't be impossible, but > it is decidedly nontrivial. That's true. Any 'bus' driver, in multipass environment, *must* export provided resources, make bus accessible and enumerate it at 'BUS_PASS_BUS', and consume resources at some later bus pass. At this time, only pcie-hotplug needs this change. But I don't see why a few lines change is "decidedly nontrivial". > >>> Or: you could skip all of that and use the fully functional mechanism >>> that already exists. >> The problem is, at least from my point of view, that we don't have it. >> The actual MIPS code doesn't work on ARM for numerous reasons. This is >> first one -> >> >> https://svnweb.freebsd.org/base/head/sys/powerpc/powerpc/nexus.c?revision=297000&view=markup#l186 >> The pre-r301453 ARM INTRNG doesn't support detached interrupts. >> And r301453 doesn't changed nothing about this. > > I'm happy to write whatever code is missing. The ARM implementation of > ofw_bus_map_intr() does seem fairly braindead and should be replaced. > > So here is where I am right now: > - The perceived advantage of the new API seems to be that the mapping > information (interrupt parent, etc.) ends up in a struct resource > instead of in a centralized mapping table True. And, in optimal case, also in RLE. See [1]. > - Additionally, it gives you a better shot at having the PIC online > before the PIC's interrupts are parsed (which is not required, but nice). Nope, we simply fount that detached pics is very dangerous and not needed. But, if you love it, it can be added as MD extension. > - Beyond these aesthetic points, there are no concrete examples of new > functionality added by this API, aside from some minor implementation > bugs of the old one on ARM that are easily fixed. Really? Or you just ignore it? > - There are, conversely, a number of concrete cases where this new API > would not be able to do the right thing. Some of these can be worked > around or fixed with significant restructuring in the MI parts of the > kernel. And i offer you a fix, without "significant restructuring in the MI parts of the kernel". Unfortunately, you did not want to accept anything other than the old API. > - If we have both, the interrupt handling code in the MI parts of the > kernel will bifurcate. This patch alone has added a bunch of #ifdef to > kern/subr_bus.c, a new file to kern/, and lots of #ifdef to dev/ofw. > This is going to be really hard to maintain if we need both. All those #ifdefs has been added as safeguards for 11/stable and they can be removed immediately. Michal > > Is this all correct? > > If so, can we please back this out until this discussion is complete? > I'm asking this formally at this point, under the Committer's Guide > section about reversion requests. > -Nathan From owner-freebsd-arch@freebsd.org Sat Jul 30 16:59:08 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 0B6B3BA7742; Sat, 30 Jul 2016 16:59:08 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from c.mail.sonic.net (c.mail.sonic.net [64.142.111.80]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id DDE1A1C93; Sat, 30 Jul 2016 16:59:07 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from zeppelin.tachypleus.net (75-101-50-44.static.sonic.net [75.101.50.44]) (authenticated bits=0) by c.mail.sonic.net (8.15.1/8.15.1) with ESMTPSA id u6UGx3oR010291 (version=TLSv1.2 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Sat, 30 Jul 2016 09:59:04 -0700 Subject: Re: INTRNG (Was: svn commit: r301453....) To: Michal Meloun References: <201606051620.u55GKD5S066398@repo.freebsd.org> <57934ABD.6010807@FreeBSD.org> <4e7a3e8f-cc21-f5f2-e3e0-4dbd554a4cd0@freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <57961549.4020105@FreeBSD.org> <57976867.6080705@FreeBSD.org> <5798E104.5020104@FreeBSD.org> <579A25BB.8070206@FreeBSD.org> <30790e40-58b4-3371-c0f0-b7545571f389@freebsd.org> <579AFFC5.1040005@FreeBSD.org> <579CD355.1050203@FreeBSD.org> Cc: "freebsd-arm@freebsd.org" , Svatopluk Kraus , "freebsd-arch@freebsd.org" From: Nathan Whitehorn Message-ID: <460fa0b3-ddb7-6247-2412-3d75a589d5e7@freebsd.org> Date: Sat, 30 Jul 2016 09:59:03 -0700 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:45.0) Gecko/20100101 Thunderbird/45.1.0 MIME-Version: 1.0 In-Reply-To: <579CD355.1050203@FreeBSD.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Sonic-CAuth: UmFuZG9tSVaNqEs/52CO7aYgImcJHsbGiejG9webnZt5xuzaFk3Rwp/vCJz2LezyE6wTS70TfhPAxexIhmzT7osYDdWf6sHyaZn3t6uv+14= X-Sonic-ID: C;pslJ63ZW5hG/eKDx2xNB0g== M;rIOY63ZW5hG/eKDx2xNB0g== X-Spam-Flag: No X-Sonic-Spam-Details: 0.0/5.0 by cerberusd X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 30 Jul 2016 16:59:08 -0000 On 07/30/16 09:18, Michal Meloun wrote: > Dne 29.07.2016 v 16:41 Nathan Whitehorn napsal(a): > [snip] > Svata is online now, so i think that he is more competent for responding > to all questions about INTRNG internals. > > >>>>> But, as you wrote, INTRNG code is "absolutely free to map and >>>>> dispatch >>>>> multiple virtual IRQs from the same shared interrupt pin" (allow me to >>>>> expand this also to INTRNG code itself) . >>>>> In attempt to simply situation abound cache, in first step, we >>>>> removed >>>>> duplicates check from above model. This also removes unpleasantly >>>>> sharing of entries. >>>>> This step converts cache to some sort of duplicate of resource list >>>>> entries. Each existing RLE have exactly one corresponding entry in >>>>> cache. >>>>> Also, the cache is not needed for INTRNG core. At this point, its >>>>> only >>>>> transfers data from ofw_bus_intr_to_rl() to consumers PIC. >>>>> >>>>> So, if we are able to attach 'key' to RLE, then we can remove the >>>>> whole >>>>> cache machinery from INTRNG. >>>>> Do you agree? >>>> No. Because you have to support two things that this still can't allow: >>>> 1. Devices attaching before their interrupt controllers. >>> You still expect that data readed from OFW must be parsed at RLE >>> creation time. I don't see single reason why we cannot postpone >>> parsing to bus_alloc_resource() time. >>> At this point, its irrelevant if bus_alloc_resource() supports >>> detached PIC or not. >> Because there are *lots* of cases in the kernel where interrupts are >> represented as a number and not a struct resource *. I've listed many >> of them; I'm sure there are more. PCI is the most notable example >> (both LSI and MSI). For both of these, the route_interrupt (LSI) and >> alloc_msi[x]() (MSI) functions give the bus driver a chance to look at >> the device tree and then they return an IRQ number. There doesn't seem >> to be any way to map that back to a struct resource * or something >> besides an internal table. >> >> Moreover, there are cases (self-added interrupts by children, for >> instance) in which the RL assigned by the parent does not reflect the >> final RL used by the child and there is no way for the parent bus to >> map an RID to a device tree entry. >> >> When we moved to doing this at interrupt assignment time rather than >> resource allocation time or another occasion on PowerPC, it was for >> these kinds of reasons. There were ways to do it later, but they were >> hugely invasive and involved changing large parts of the kernel. Even >> then, it would still be fragile: Trying to guess what device tree >> entry was meant at bus_alloc_resource() time is much more error-prone >> than doing the mapping when reading that device tree to assign the >> resources in the first place, when you can make guarantees. >> >>>> 2. Interrupts encoded entirely in a 32-bit integer rather than a >>>> struct resource * in a shared rman. This is required to support PCI >>>> (both LSI and MSI). >>>> >>>> #2 could of course be fixed by completely retooling the API of the PCI >>>> bus code, but, since it fixes something that currently is not a >>>> problem, why would we do that? >>> All relevant bus functions have struct resource * as an argument. And, >>> in post r301453 kernel, all necessary data are attached to it. >>> I don't see any reason for API change. >> There is: >> int PCIB_ROUTE_INTERRUPT(device_t, device_t, pin) >> >> Which takes a PIN (A, B, C, D) and returns the IRQ to add to the RL. >> This is the only real opportunity to parse an interrupt-map and does >> not deal with struct resource. >> >> There are also PCIB_ALLOC_MSI[X], which take a device_t and count and >> return a vector of IRQ numbers. >> >> How would you do resource allocation given these APIs? There seem to >> be some dubious-looking attempts at generic MSI mapping code in the >> new kern/subr_intr.c that don't allow the specification of more than >> 32-bit interrupt specifiers and otherwise exactly duplicate the >> pre-301453 flow for MSIs, but in a more complicated and >> less-functional way. >> >> (While investigating this, I also just found >> dev/pci/pci_host_generic.c, which mostly does the wrong things, is >> actually specific to a couple of ARM boards, and duplicates code in >> dev/ofw/ofwpci.c. But that's another discussion...) > [snip][2] >>>>>>> Each OFW device has a lot of dependencies. It must enable clocks, >>>>>>> power >>>>>>> regulators..., it must be taken from reset. All these action must be >>>>>>> done before we can access single register. >>>>>>> Most of these action can be done only with attached >>>>>>> clock/power/reset >>>>>>> controller. >>>>>>> Within this, interrupts are not special. >>>>>> They are actually quite special in that the kernel enables them late >>>>>> and so you can defer setup. They are also special in that, for that >>>>>> reason, it is possible to design systems with circular dependency >>>>>> graphs with interrupts. It is not possible to do this with, for >>>>>> example, clocks: if I need to apply a clock to the clock controller >>>>>> with itself, the system is just not bootable and such a system will >>>>>> not be built or shipped. Interrupts need only happen later, after >>>>>> device setup, and so such systems *are* designed and shipped. The >>>>>> same >>>>>> is true for power. >>>>> The G5 problem is standard 'cross type' circular dependency. You >>>>> must >>>>> (at BUS_PASS_BUS) initialize memory address window of PCI bridge and >>>>> start bus enumeration. >>>>> Rest of PCI bridge initialization (including all IRQ stuff) must be >>>>> postponed to any later pass. Of course, all interrupt controllers must >>>>> be attached at BUS_PASS_INTERRUPT. >>>>> Or I missed something? >>>> Yes. As I said, you could solve it that way. It would, of course, >>>> require a massive retooling of newbus (to allow partial delayed >>>> attach), everything in dev/pci (which doesn't expect that), and >>>> (almost) every PCI driver in and out of the tree. >>> I still think that only host to PCI bridge must me modified, and PCI-PCI >>> bridge must be run at BUS_PASS_BUS (which is single line modification). >> On Powermacs, one PIC is 5 levels deep in the bus hierarchy, under 3 >> PCI<->PCI bridges and a device that also has an ATA controller and a >> bunch of other things. > Yes, I see. And, as a said before, it's pretty common situation in OFW > based world. > Imho, all whats is needed is: > https://github.com/strejda/freebsd/commit/96ab64e04dcebb131643c2ae1cf373194d23e8e5 > (of course, it's not tested, I havn't access to powermac HW) That would probably work on at least some of the hardware. Some of those parents can have their own interrupts, however, which would break. I'll investigate how often this happens. > It's hard for me to understand why you say that this change requires > "massive retooling of newbus, everything in dev/pci, every PCI driver). > >> Some of those can have their own interrupts (which is fairly common >> for error reporting or PCI-E hotplug). It wouldn't be impossible, but >> it is decidedly nontrivial. > That's true. Any 'bus' driver, in multipass environment, *must* export > provided resources, make bus accessible and enumerate it at > 'BUS_PASS_BUS', and > consume resources at some later bus pass. > At this time, only pcie-hotplug needs this change. But I don't see why > a few lines change is "decidedly nontrivial". It's not just a few lines change. Newbus provides no mechanism for a bus to attach at two different bus passes. >>>> Or: you could skip all of that and use the fully functional mechanism >>>> that already exists. >>> The problem is, at least from my point of view, that we don't have it. >>> The actual MIPS code doesn't work on ARM for numerous reasons. This is >>> first one -> >>> >>> https://svnweb.freebsd.org/base/head/sys/powerpc/powerpc/nexus.c?revision=297000&view=markup#l186 >>> The pre-r301453 ARM INTRNG doesn't support detached interrupts. >>> And r301453 doesn't changed nothing about this. >> I'm happy to write whatever code is missing. The ARM implementation of >> ofw_bus_map_intr() does seem fairly braindead and should be replaced. >> >> So here is where I am right now: >> - The perceived advantage of the new API seems to be that the mapping >> information (interrupt parent, etc.) ends up in a struct resource >> instead of in a centralized mapping table > True. And, in optimal case, also in RLE. See [1]. Your code also has a centralized table (static struct intr_irqsrc *irq_sources[NIRQ];), basically the same one, so I'm actually really confused on this point. The only actual difference seems to be that the firmware interrupt descriptor is stored in the RLE at bus_alloc_resource() time instead of in the table at bus enumeration time and that the new code requires some extra bus methods. >> - Additionally, it gives you a better shot at having the PIC online >> before the PIC's interrupts are parsed (which is not required, but nice). > Nope, we simply fount that detached pics is very dangerous and not > needed. But, if you love it, it can be added as MD extension. Dangerous how? I don't "love it"; it's an unfortunate necessity. >> - Beyond these aesthetic points, there are no concrete examples of new >> functionality added by this API, aside from some minor implementation >> bugs of the old one on ARM that are easily fixed. > Really? Or you just ignore it? Which ones? I've been asking for a week and a half now and you have responded with some hypothetical examples, all of which either (a) do not seem to occur in the real world and (b) were trivially implementable with the existing code. > >> - There are, conversely, a number of concrete cases where this new API >> would not be able to do the right thing. Some of these can be worked >> around or fixed with significant restructuring in the MI parts of the >> kernel. > And i offer you a fix, without "significant restructuring in the MI > parts of the kernel". Unfortunately, you did not want to accept anything > other than the old API. For one of them, partially. For the others, not so much. For example, I have no idea how to implement PCI interrupts (PCIB_ROUTE_INTERRUPT) in this framework. I am perfectly happy to accept a new API. What I am not perfectly happy to accept is an API that makes it impossible for me to support the platforms that I need to support and that, simultaneously, doesn't even provide any new capabilities on other platforms. The current API is certainly a bit of a hack and I would be pleased to see it go; but the replacement needs to be more functional and more robust. As far as I can tell, I literally *cannot* move PowerPC over to this architecture. The ideal API would be one in which the resource enumeration code could assign, instead of numbers, some structured information that contains the full firmware specifier (string parent + interrupt pin for ACPI, interrupt parent phandle + specifier for device tree, bare numbers for simple systems). That seemed, and seems, too invasive. I think we agree on this and have chosen to approximate that API in two different ways. When we designed the interrupt mapping code, we evaluated a large number of possible approaches. Something very much like this was one of them. It was rejected as being too fragile (it requires resource allocation and enumeration to be coupled) and to have too many corner cases (the PCI MSI and LSI APIs, for instance). The interrupt mapping stuff seemed at the time, and so far still seems, like the least-bad approximation to the ideal case: it is essentially that ideal API, in that it happens at bus enumeration and involves just assigning the firmware specifiers, but with lookup keys to that information stuffed into the 32-bit numbers that the rest of the system uses. >> - If we have both, the interrupt handling code in the MI parts of the >> kernel will bifurcate. This patch alone has added a bunch of #ifdef to >> kern/subr_bus.c, a new file to kern/, and lots of #ifdef to dev/ofw. >> This is going to be really hard to maintain if we need both. > All those #ifdefs has been added as safeguards for 11/stable and they > can be removed immediately. They should not be and I will request immediate reversion and appeal to core@ if they are before this discussion is complete. -Nathan > Michal > >> Is this all correct? >> >> If so, can we please back this out until this discussion is complete? >> I'm asking this formally at this point, under the Committer's Guide >> section about reversion requests. >> -Nathan > _______________________________________________ > freebsd-arch@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/freebsd-arch > To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" > From owner-freebsd-arch@freebsd.org Sat Jul 30 18:06:32 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 99F60BA894E for ; Sat, 30 Jul 2016 18:06:32 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-it0-x22b.google.com (mail-it0-x22b.google.com [IPv6:2607:f8b0:4001:c0b::22b]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 55D5E1B15 for ; Sat, 30 Jul 2016 18:06:32 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-it0-x22b.google.com with SMTP id j124so136379099ith.1 for ; Sat, 30 Jul 2016 11:06:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=DNE2xl5AL68CunojHSXzb7aIZvbRL9t61wTxvWudrF8=; b=vwTez8euEf9PneaDNPkBk28NZMYCS1BMgUAeEV+cnS7b0zIaGjtLyVdDN39f3AUyEf 2MGq8Vi5Vey/DpYZWPTTpVGC/jWa+43lcf5TJzl/SOlcP/Q9rAPERLsfUmOAfVjWMYPb FNFcNZSrau5dR/CRKIG0TP33ypfgdlzpaYgxHAjwb7Tz954IgD89w4fT8YGijrehKLX9 nVY8GsFRR9Ch3lVyt2CzHjAamkjyzaz4SBOY7gyPGh/fcCj1V0be4RLaOLQDOKRUX5Br SqudUPv85NFnmH9LxQld0Cj1FxL8pdZ4sUlKB7xRBjVaC+pqU34fnP3qo4eAsTyfPkp+ 6G2w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=DNE2xl5AL68CunojHSXzb7aIZvbRL9t61wTxvWudrF8=; b=jdpcAEMK5yHbO98r6D/nFnqb+gAGOASAseMGVxXDP5GCzIkPmpHYZoohV2/BNd+j6v IFIMHKTF9D60z7N1ENWBQLkhvz1rr9tPzgLjBdzHRUUw3rCEIS/mOJQCb0ut4xAHMWo5 KGsiq7747FXj0bE9lUrZfZFtixJKPcBq1UOx5IUMH4yjVuvRXtpyI8GAX8uVqXBYdc8S 9+VGlhjTrnGx6bXrNYFCKWKqlClvphOyY7XQP9F9vzFOJN6U7MB9uFBxr4l01RWfgvEt kkjSJ18OZANhCjUNl+Z9uxHWJfh1gmP44lXBWq4YrEz+D8eE8HJiYwPnc9d/r+q5wI0x Jowg== X-Gm-Message-State: AEkoouvzl076Yt+/X/yKk+bNFDJ2XDmWjgSuCXjH0gIg6/8SFxe7u7jQk2cK0ipJgNY/2AwV6hVETemhJjLiJw== X-Received: by 10.36.16.138 with SMTP id 132mr7311518ity.60.1469901991766; Sat, 30 Jul 2016 11:06:31 -0700 (PDT) MIME-Version: 1.0 Sender: wlosh@bsdimp.com Received: by 10.79.137.131 with HTTP; Sat, 30 Jul 2016 11:06:30 -0700 (PDT) X-Originating-IP: [50.253.99.174] In-Reply-To: <460fa0b3-ddb7-6247-2412-3d75a589d5e7@freebsd.org> References: <201606051620.u55GKD5S066398@repo.freebsd.org> <57934ABD.6010807@FreeBSD.org> <4e7a3e8f-cc21-f5f2-e3e0-4dbd554a4cd0@freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <57961549.4020105@FreeBSD.org> <57976867.6080705@FreeBSD.org> <5798E104.5020104@FreeBSD.org> <579A25BB.8070206@FreeBSD.org> <30790e40-58b4-3371-c0f0-b7545571f389@freebsd.org> <579AFFC5.1040005@FreeBSD.org> <579CD355.1050203@FreeBSD.org> <460fa0b3-ddb7-6247-2412-3d75a589d5e7@freebsd.org> From: Warner Losh Date: Sat, 30 Jul 2016 12:06:30 -0600 X-Google-Sender-Auth: blMql6EKvNas1ndlZIGB-cpG4_0 Message-ID: Subject: Re: INTRNG (Was: svn commit: r301453....) To: Nathan Whitehorn Cc: Michal Meloun , "freebsd-arm@freebsd.org" , Svatopluk Kraus , "freebsd-arch@freebsd.org" Content-Type: text/plain; charset=UTF-8 X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 30 Jul 2016 18:06:32 -0000 On Sat, Jul 30, 2016 at 10:59 AM, Nathan Whitehorn wrote: > It's not just a few lines change. Newbus provides no mechanism for a bus to > attach at two different bus passes. Sure it does. That's used in the Atmel code (which might not be in the tree) for some things. Newbus lets you know for each new pass that something is happening. Perhaps you could be clearer as to what exactly it doesn't provide. Warner From owner-freebsd-arch@freebsd.org Sat Jul 30 18:54:04 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id A0F47BA92C3; Sat, 30 Jul 2016 18:54:04 +0000 (UTC) (envelope-from mmel@FreeBSD.org) Received: from mail.miracle.cz (mail.miracle.cz [193.84.128.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mail.miracle.cz", Issuer "Miracle Group Root CA" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 57B721CB1; Sat, 30 Jul 2016 18:54:04 +0000 (UTC) (envelope-from mmel@FreeBSD.org) Received: from [193.84.128.50] (meloun.ad.miracle.cz [193.84.128.50]) by mail.miracle.cz (Postfix) with ESMTPSA id 135D43AC9C; Sat, 30 Jul 2016 20:54:01 +0200 (CEST) Subject: Re: INTRNG (Was: svn commit: r301453....) To: Nathan Whitehorn References: <201606051620.u55GKD5S066398@repo.freebsd.org> <5794720F.4050303@FreeBSD.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <57961549.4020105@FreeBSD.org> <57976867.6080705@FreeBSD.org> <5798E104.5020104@FreeBSD.org> <579A25BB.8070206@FreeBSD.org> <30790e40-58b4-3371-c0f0-b7545571f389@freebsd.org> <579AFFC5.1040005@FreeBSD.org> <579CD355.1050203@FreeBSD.org> <460fa0b3-ddb7-6247-2412-3d75a589d5e7@freebsd.org> Cc: "freebsd-arm@freebsd.org" , Svatopluk Kraus , "freebsd-arch@freebsd.org" From: Michal Meloun X-Enigmail-Draft-Status: N1110 Message-ID: <579CF7C8.1040302@FreeBSD.org> Date: Sat, 30 Jul 2016 20:54:00 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <460fa0b3-ddb7-6247-2412-3d75a589d5e7@freebsd.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.4.3 (mail.miracle.cz); Sat, 30 Jul 2016 20:54:01 +0200 (CEST) X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 30 Jul 2016 18:54:04 -0000 Dne 30.07.2016 v 18:59 Nathan Whitehorn napsal(a): > > > On 07/30/16 09:18, Michal Meloun wrote: >> Dne 29.07.2016 v 16:41 Nathan Whitehorn napsal(a): >> [snip] >> Svata is online now, so i think that he is more competent for responding >> to all questions about INTRNG internals. >> >> >>>>>> But, as you wrote, INTRNG code is "absolutely free to map and >>>>>> dispatch >>>>>> multiple virtual IRQs from the same shared interrupt pin" (allow >>>>>> me to >>>>>> expand this also to INTRNG code itself) . >>>>>> In attempt to simply situation abound cache, in first step, we >>>>>> removed >>>>>> duplicates check from above model. This also removes unpleasantly >>>>>> sharing of entries. >>>>>> This step converts cache to some sort of duplicate of resource list >>>>>> entries. Each existing RLE have exactly one corresponding entry in >>>>>> cache. >>>>>> Also, the cache is not needed for INTRNG core. At this point, its >>>>>> only >>>>>> transfers data from ofw_bus_intr_to_rl() to consumers PIC. >>>>>> >>>>>> So, if we are able to attach 'key' to RLE, then we can remove the >>>>>> whole >>>>>> cache machinery from INTRNG. >>>>>> Do you agree? >>>>> No. Because you have to support two things that this still can't >>>>> allow: >>>>> 1. Devices attaching before their interrupt controllers. >>>> You still expect that data readed from OFW must be parsed at RLE >>>> creation time. I don't see single reason why we cannot postpone >>>> parsing to bus_alloc_resource() time. >>>> At this point, its irrelevant if bus_alloc_resource() supports >>>> detached PIC or not. >>> Because there are *lots* of cases in the kernel where interrupts are >>> represented as a number and not a struct resource *. I've listed many >>> of them; I'm sure there are more. PCI is the most notable example >>> (both LSI and MSI). For both of these, the route_interrupt (LSI) and >>> alloc_msi[x]() (MSI) functions give the bus driver a chance to look at >>> the device tree and then they return an IRQ number. There doesn't seem >>> to be any way to map that back to a struct resource * or something >>> besides an internal table. >>> >>> Moreover, there are cases (self-added interrupts by children, for >>> instance) in which the RL assigned by the parent does not reflect the >>> final RL used by the child and there is no way for the parent bus to >>> map an RID to a device tree entry. >>> >>> When we moved to doing this at interrupt assignment time rather than >>> resource allocation time or another occasion on PowerPC, it was for >>> these kinds of reasons. There were ways to do it later, but they were >>> hugely invasive and involved changing large parts of the kernel. Even >>> then, it would still be fragile: Trying to guess what device tree >>> entry was meant at bus_alloc_resource() time is much more error-prone >>> than doing the mapping when reading that device tree to assign the >>> resources in the first place, when you can make guarantees. >>> >>>>> 2. Interrupts encoded entirely in a 32-bit integer rather than a >>>>> struct resource * in a shared rman. This is required to support PCI >>>>> (both LSI and MSI). >>>>> >>>>> #2 could of course be fixed by completely retooling the API of the >>>>> PCI >>>>> bus code, but, since it fixes something that currently is not a >>>>> problem, why would we do that? >>>> All relevant bus functions have struct resource * as an argument. And, >>>> in post r301453 kernel, all necessary data are attached to it. >>>> I don't see any reason for API change. >>> There is: >>> int PCIB_ROUTE_INTERRUPT(device_t, device_t, pin) >>> >>> Which takes a PIN (A, B, C, D) and returns the IRQ to add to the RL. >>> This is the only real opportunity to parse an interrupt-map and does >>> not deal with struct resource. >>> >>> There are also PCIB_ALLOC_MSI[X], which take a device_t and count and >>> return a vector of IRQ numbers. >>> >>> How would you do resource allocation given these APIs? There seem to >>> be some dubious-looking attempts at generic MSI mapping code in the >>> new kern/subr_intr.c that don't allow the specification of more than >>> 32-bit interrupt specifiers and otherwise exactly duplicate the >>> pre-301453 flow for MSIs, but in a more complicated and >>> less-functional way. >>> >>> (While investigating this, I also just found >>> dev/pci/pci_host_generic.c, which mostly does the wrong things, is >>> actually specific to a couple of ARM boards, and duplicates code in >>> dev/ofw/ofwpci.c. But that's another discussion...) >> [snip][2] >>>>>>>> Each OFW device has a lot of dependencies. It must enable clocks, >>>>>>>> power >>>>>>>> regulators..., it must be taken from reset. All these action >>>>>>>> must be >>>>>>>> done before we can access single register. >>>>>>>> Most of these action can be done only with attached >>>>>>>> clock/power/reset >>>>>>>> controller. >>>>>>>> Within this, interrupts are not special. >>>>>>> They are actually quite special in that the kernel enables them >>>>>>> late >>>>>>> and so you can defer setup. They are also special in that, for that >>>>>>> reason, it is possible to design systems with circular dependency >>>>>>> graphs with interrupts. It is not possible to do this with, for >>>>>>> example, clocks: if I need to apply a clock to the clock controller >>>>>>> with itself, the system is just not bootable and such a system will >>>>>>> not be built or shipped. Interrupts need only happen later, after >>>>>>> device setup, and so such systems *are* designed and shipped. The >>>>>>> same >>>>>>> is true for power. >>>>>> The G5 problem is standard 'cross type' circular dependency. You >>>>>> must >>>>>> (at BUS_PASS_BUS) initialize memory address window of PCI bridge >>>>>> and >>>>>> start bus enumeration. >>>>>> Rest of PCI bridge initialization (including all IRQ stuff) must be >>>>>> postponed to any later pass. Of course, all interrupt controllers >>>>>> must >>>>>> be attached at BUS_PASS_INTERRUPT. >>>>>> Or I missed something? >>>>> Yes. As I said, you could solve it that way. It would, of course, >>>>> require a massive retooling of newbus (to allow partial delayed >>>>> attach), everything in dev/pci (which doesn't expect that), and >>>>> (almost) every PCI driver in and out of the tree. >>>> I still think that only host to PCI bridge must me modified, and >>>> PCI-PCI >>>> bridge must be run at BUS_PASS_BUS (which is single line >>>> modification). >>> On Powermacs, one PIC is 5 levels deep in the bus hierarchy, under 3 >>> PCI<->PCI bridges and a device that also has an ATA controller and a >>> bunch of other things. >> Yes, I see. And, as a said before, it's pretty common situation in OFW >> based world. >> Imho, all whats is needed is: >> https://github.com/strejda/freebsd/commit/96ab64e04dcebb131643c2ae1cf373194d23e8e5 >> >> (of course, it's not tested, I havn't access to powermac HW) > > That would probably work on at least some of the hardware. Some of > those parents can have their own interrupts, however, which would > break. I'll investigate how often this happens. > >> It's hard for me to understand why you say that this change requires >> "massive retooling of newbus, everything in dev/pci, every PCI >> driver). >> >>> Some of those can have their own interrupts (which is fairly common >>> for error reporting or PCI-E hotplug). It wouldn't be impossible, but >>> it is decidedly nontrivial. >> That's true. Any 'bus' driver, in multipass environment, *must* export >> provided resources, make bus accessible and enumerate it at >> 'BUS_PASS_BUS', and >> consume resources at some later bus pass. >> At this time, only pcie-hotplug needs this change. But I don't see why >> a few lines change is "decidedly nontrivial". > > It's not just a few lines change. Newbus provides no mechanism for a > bus to attach at two different bus passes. ?? Please see: https://github.com/strejda/tegra/blob/master/sys/arm/nvidia/drm2/tegra_host1x.c#L639 and https://github.com/strejda/tegra/blob/master/sys/arm/nvidia/drm2/tegra_host1x.c#L451 > >>>>> Or: you could skip all of that and use the fully functional mechanism >>>>> that already exists. >>>> The problem is, at least from my point of view, that we don't have it. >>>> The actual MIPS code doesn't work on ARM for numerous reasons. >>>> This is >>>> first one -> >>>> >>>> https://svnweb.freebsd.org/base/head/sys/powerpc/powerpc/nexus.c?revision=297000&view=markup#l186 >>>> The pre-r301453 ARM INTRNG doesn't support detached interrupts. >>>> And r301453 doesn't changed nothing about this. >>> I'm happy to write whatever code is missing. The ARM implementation of >>> ofw_bus_map_intr() does seem fairly braindead and should be replaced. >>> >>> So here is where I am right now: >>> - The perceived advantage of the new API seems to be that the mapping >>> information (interrupt parent, etc.) ends up in a struct resource >>> instead of in a centralized mapping table >> True. And, in optimal case, also in RLE. See [1]. > > Your code also has a centralized table (static struct intr_irqsrc > *irq_sources[NIRQ];), basically the same one, so I'm actually really > confused on this point. The only actual difference seems to be that > the firmware interrupt descriptor is stored in the RLE at > bus_alloc_resource() time instead of in the table at bus enumeration > time and that the new code requires some extra bus methods. The irq_sources holds only real interrupts (all interrupts pin from all interrupts controllers). Your effort is move all 'interrupt description data' out of INTRNG code - to RLE (at bus enumeration time) (see [1]) and to struct resource (at bus_alloc_resource() time). The new bus method is only temporary shortcut to bypass 11 release. But, to be fair, I was hoping it might be necessary. [1] This is last missing step in whole patch series and, in optimal case, it need new filed(s) in RLE structure. So we chose to postpone discussion about this to time after 11 release. > >>> - Additionally, it gives you a better shot at having the PIC online >>> before the PIC's interrupts are parsed (which is not required, but >>> nice). >> Nope, we simply fount that detached pics is very dangerous and not >> needed. But, if you love it, it can be added as MD extension. > > Dangerous how? I don't "love it"; it's an unfortunate necessity. Imho, some drivers may rely on correct return code from bus_alloc_resource() or from bus_setup_intr(). For example to detect if interrupt exist, or if given interrupt configuration is supported. > >>> - Beyond these aesthetic points, there are no concrete examples of new >>> functionality added by this API, aside from some minor implementation >>> bugs of the old one on ARM that are easily fixed. >> Really? Or you just ignore it? > > Which ones? I've been asking for a week and a half now and you have > responded with some hypothetical examples, all of which either (a) do > not seem to occur in the real world and (b) were trivially > implementable with the existing code. > >> >>> - There are, conversely, a number of concrete cases where this new API >>> would not be able to do the right thing. Some of these can be worked >>> around or fixed with significant restructuring in the MI parts of the >>> kernel. >> And i offer you a fix, without "significant restructuring in the MI >> parts of the kernel". Unfortunately, you did not want to accept anything >> other than the old API. > > For one of them, partially. For the others, not so much. For example, > I have no idea how to implement PCI interrupts (PCIB_ROUTE_INTERRUPT) > in this framework. I am perfectly happy to accept a new API. What I am > not perfectly happy to accept is an API that makes it impossible for > me to support the platforms that I need to support and that, > simultaneously, doesn't even provide any new capabilities on other > platforms. >From one of previous mail: But again, all what I want in this area is (for me) simple change of your ofw_bus_map_intr() method to something like: enum intr_map_data_type { INTR_MAP_DATA_OFW, INTR_MAP_DATA_GPIO, ... }; int bus_map_intr(device_t dev, enum intr_map_data_type type, uintptr_t pic_id, void *config_data, int config_size) > The current API is certainly a bit of a hack and I would be pleased to > see it go; but the replacement needs to be more functional and more > robust. As far as I can tell, I literally *cannot* move PowerPC over > to this architecture. Yes, at this time, I agree. If you can accept enhancement of (owf_)bus_map_intr() then we can move to next, significantly less hackish, state. > The ideal API would be one in which the resource enumeration code > could assign, instead of numbers, some structured information that > contains the full firmware specifier (string parent + interrupt pin > for ACPI, interrupt parent phandle + specifier for device tree, bare > numbers for simple systems). Yes, exactly. I am happy that at least we agree on something. The only difference is that we want triplet or > That seemed, and seems, too invasive. I think we agree on this and > have chosen to approximate that API in two different ways. I still think that we found way how we can realize the above idea in non-invasive way. >From my point of view the solution is: Put/attach the above triplet to RLE and then copy it (within bus_resource_attach() to struct resource. But yes, I understand that I am not able to explain clearly enough. > > When we designed the interrupt mapping code, we evaluated a large > number of possible approaches. Something very much like this was one > of them. It was rejected as being too fragile (it requires resource > allocation and enumeration to be coupled) and to have too many corner > cases (the PCI MSI and LSI APIs, for instance). The interrupt mapping > stuff seemed at the time, and so far still seems, like the least-bad > approximation to the ideal case: it is essentially that ideal API, in > that it happens at bus enumeration and involves just assigning the > firmware specifiers, but with lookup keys to that information stuffed > into the 32-bit numbers that the rest of the system uses. The raw PCI (or MSIX) case is relatively simple. The PCI domain uses raw numbers and it's host-to-pci bridge job to translate raw numbers from PCI domain to (for example) OFW based resource. At this point, I'm still not sure about MSI... Michal >>> - If we have both, the interrupt handling code in the MI parts of the >>> kernel will bifurcate. This patch alone has added a bunch of #ifdef to >>> kern/subr_bus.c, a new file to kern/, and lots of #ifdef to dev/ofw. >>> This is going to be really hard to maintain if we need both. >> All those #ifdefs has been added as safeguards for 11/stable and they >> can be removed immediately. > > They should not be and I will request immediate reversion and appeal > to core@ if they are before this discussion is complete. > -Nathan > >> Michal >> >>> Is this all correct? >>> >>> If so, can we please back this out until this discussion is complete? >>> I'm asking this formally at this point, under the Committer's Guide >>> section about reversion requests. >>> -Nathan >> _______________________________________________ >> freebsd-arch@freebsd.org mailing list >> https://lists.freebsd.org/mailman/listinfo/freebsd-arch >> To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" >> > > _______________________________________________ > freebsd-arch@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/freebsd-arch > To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" From owner-freebsd-arch@freebsd.org Sat Jul 30 19:26:12 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 156C9BA9D49; Sat, 30 Jul 2016 19:26:12 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from d.mail.sonic.net (d.mail.sonic.net [64.142.111.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id D05931F2C; Sat, 30 Jul 2016 19:26:11 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from zeppelin.tachypleus.net (75-101-50-44.static.sonic.net [75.101.50.44]) (authenticated bits=0) by d.mail.sonic.net (8.15.1/8.15.1) with ESMTPSA id u6UJQ2dD027948 (version=TLSv1.2 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Sat, 30 Jul 2016 12:26:03 -0700 Subject: Re: INTRNG (Was: svn commit: r301453....) To: Warner Losh References: <201606051620.u55GKD5S066398@repo.freebsd.org> <8bfd8668-bc49-e109-e610-b5cd470be3ec@freebsd.org> <57950005.6070403@FreeBSD.org> <57961549.4020105@FreeBSD.org> <57976867.6080705@FreeBSD.org> <5798E104.5020104@FreeBSD.org> <579A25BB.8070206@FreeBSD.org> <30790e40-58b4-3371-c0f0-b7545571f389@freebsd.org> <579AFFC5.1040005@FreeBSD.org> <579CD355.1050203@FreeBSD.org> <460fa0b3-ddb7-6247-2412-3d75a589d5e7@freebsd.org> Cc: Michal Meloun , "freebsd-arm@freebsd.org" , Svatopluk Kraus , "freebsd-arch@freebsd.org" From: Nathan Whitehorn Message-ID: <3d17565e-8831-295a-46be-6341e17a8e6b@freebsd.org> Date: Sat, 30 Jul 2016 12:26:02 -0700 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:45.0) Gecko/20100101 Thunderbird/45.1.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Sonic-CAuth: UmFuZG9tSVYLmf2grmlq2PiNmyOV9dAUyln7hd83lNxrQFynRaMakcpYmtQWfOwdzhKNaSQU5jywvaBcpbGFBrUqEoRrRvx8qtoqPTc3IGA= X-Sonic-ID: C;5DEQdItW5hG6eq/hcgQksw== M;mthndItW5hG6eq/hcgQksw== X-Spam-Flag: No X-Sonic-Spam-Details: 0.0/5.0 by cerberusd X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 30 Jul 2016 19:26:12 -0000 On 07/30/16 11:06, Warner Losh wrote: > On Sat, Jul 30, 2016 at 10:59 AM, Nathan Whitehorn > wrote: >> It's not just a few lines change. Newbus provides no mechanism for a bus to >> attach at two different bus passes. > Sure it does. That's used in the Atmel code (which might not be in the tree) for > some things. Newbus lets you know for each new pass that something is happening. > > Perhaps you could be clearer as to what exactly it doesn't provide. > > Warner > interesting. I hadn't realized that. We'd need to find all the bus drivers in the tree and make them attach this way, I guess. There are still some ordering dilemmas here coming from the static nature of the ordering. For example, if you have two PICs supported by the same driver, one of which is attached to the other, how do you order their attachments? I have a whole list of these at https://wiki.freebsd.org/Complicated_Interrupts. The list is basically the full set of weird cases we support with the current code. It would be great to get some responses to how you would implement those specific things with this new API. As usual, there are two things I would like to see: 1. A case of something that wasn't supported by the old API, but is supported by the new API. 2. How I would support all of the complicated cases the existing code supports, listed on the wiki, with the new API. -Nathan