From owner-freebsd-arch@freebsd.org Thu Jul 23 17:32:15 2015 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 608D69A9A54 for ; Thu, 23 Jul 2015 17:32:15 +0000 (UTC) (envelope-from andrew@fubar.geek.nz) 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 4B8351B58 for ; Thu, 23 Jul 2015 17:32:15 +0000 (UTC) (envelope-from andrew@fubar.geek.nz) Received: by mailman.ysv.freebsd.org (Postfix) id 4A4259A9A53; Thu, 23 Jul 2015 17:32:15 +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 49C2D9A9A52 for ; Thu, 23 Jul 2015 17:32:15 +0000 (UTC) (envelope-from andrew@fubar.geek.nz) Received: from kif.fubar.geek.nz (kif.fubar.geek.nz [178.62.119.249]) by mx1.freebsd.org (Postfix) with ESMTP id 192AA1B55 for ; Thu, 23 Jul 2015 17:32:14 +0000 (UTC) (envelope-from andrew@fubar.geek.nz) Received: from bender (bender.sec.cl.cam.ac.uk [IPv6:2001:630:212:2a8:4e72:b9ff:fe93:61bf]) by kif.fubar.geek.nz (Postfix) with ESMTPSA id A0927D7A21 for ; Thu, 23 Jul 2015 17:24:50 +0000 (UTC) Date: Thu, 23 Jul 2015 18:24:49 +0100 From: Andrew Turner To: arch@FreeBSD.org Subject: Interrupts on all CPUs Message-ID: <20150723182449.0ecc7ea9@bender> X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.27; amd64-portbld-freebsd10.0) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="MP_/yGe585u0HCR6ngMWAQOHcw/" X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 23 Jul 2015 17:32:15 -0000 --MP_/yGe585u0HCR6ngMWAQOHcw/ Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Content-Disposition: inline We have a problem on ARM where some interrupts need to be unmasked on all CPUs in an SMP environment. The main place we need this is with the timer driver as the hardware can send interrupts to the requested CPU. Until now we have hacked around this by unmasking the three interrupts the timer hardware uses when we enable the non-boot CPU. Luckily these interrupts values have been common across all SoCs, however there is no requirement for this to be the case. To fix this I'm proposing adding a flag to bus_setup_intr to add support for per-cpu interrupts. The architecture code is then expected to perform whatever is needed to handle this. I have attached a proof of concept patch that adds support to arm64 for this. When we setup an interrupt with the flag set it will record this so when the non-boot CPUs are enabled they can unmask the interrupt. It will then signal to any running CPUs to unmask the given interrupt. I would expect this could be extended to other architectures, however I only know the arm and arm64 code, and am only able to test there. Andrew --MP_/yGe585u0HCR6ngMWAQOHcw/ Content-Type: text/x-patch Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename=0001-WIP-Allow-per-cpu-interrupts.patch >From 70c0be107164ec093da3da8357654fa256014fc7 Mon Sep 17 00:00:00 2001 From: Andrew Turner Date: Thu, 23 Jul 2015 10:43:13 +0100 Subject: [PATCH] WIP: Allow per-cpu interrupts This lets us flag interrupts as being needed to be enabled on all cpus. The main use for this on ARM is to enable the timer interrupts so it will be enabled when secondary cores are enabled. --- sys/arm/arm/generic_timer.c | 5 +++-- sys/arm64/arm64/gic.c | 7 ------- sys/arm64/arm64/intr_machdep.c | 24 +++++++++++++++++++++++- sys/sys/bus.h | 1 + 4 files changed, 27 insertions(+), 10 deletions(-) diff --git a/sys/arm/arm/generic_timer.c b/sys/arm/arm/generic_timer.c index 636ba75..540bf2e 100644 --- a/sys/arm/arm/generic_timer.c +++ b/sys/arm/arm/generic_timer.c @@ -373,8 +373,9 @@ arm_tmr_attach(device_t dev) /* Setup secure, non-secure and virtual IRQs handler */ for (i = 0; i < 3; i++) { - error = bus_setup_intr(dev, sc->res[i], INTR_TYPE_CLK, - arm_tmr_intr, NULL, sc, &sc->ihl[i]); + error = bus_setup_intr(dev, sc->res[i], + INTR_TYPE_CLK | INTR_PERCPU, arm_tmr_intr, NULL, sc, + &sc->ihl[i]); if (error) { device_printf(dev, "Unable to alloc int resource.\n"); return (ENXIO); diff --git a/sys/arm64/arm64/gic.c b/sys/arm64/arm64/gic.c index 7c0692e..98e79d4 100644 --- a/sys/arm64/arm64/gic.c +++ b/sys/arm64/arm64/gic.c @@ -143,13 +143,6 @@ gic_init_secondary(device_t dev) /* Enable interrupt distribution */ gic_d_write_4(sc, GICD_CTLR, 0x01); - - /* - * Activate the timer interrupts: virtual, secure, and non-secure. - */ - gic_d_write_4(sc, GICD_ISENABLER(27 >> 5), (1UL << (27 & 0x1F))); - gic_d_write_4(sc, GICD_ISENABLER(29 >> 5), (1UL << (29 & 0x1F))); - gic_d_write_4(sc, GICD_ISENABLER(30 >> 5), (1UL << (30 & 0x1F))); } #endif diff --git a/sys/arm64/arm64/intr_machdep.c b/sys/arm64/arm64/intr_machdep.c index 7c383ee..e1f912a 100644 --- a/sys/arm64/arm64/intr_machdep.c +++ b/sys/arm64/arm64/intr_machdep.c @@ -81,6 +81,7 @@ struct arm64_intr_entry { enum intr_trigger i_trig; enum intr_polarity i_pol; + bool i_percpu; u_int i_hw_irq; /* Physical interrupt number */ u_int i_cntidx; /* Index in intrcnt table */ u_int i_handlers; /* Allocated handlers */ @@ -155,6 +156,7 @@ intr_acquire(u_int hw_irq) intr->i_handlers = 0; intr->i_trig = INTR_TRIGGER_CONFORM; intr->i_pol = INTR_POLARITY_CONFORM; + intr->i_percpu = false; intr->i_cntidx = atomic_fetchadd_int(&intrcntidx, 1); intr->i_cntp = &intrcnt[intr->i_cntidx]; intr->i_hw_irq = hw_irq; @@ -312,6 +314,17 @@ arm_enable_intr(void) return (0); } +#ifdef SMP +static void +unmask_irq(void *i) +{ + struct arm64_intr_entry *intr; + + intr = i; + PIC_UNMASK(root_pic, intr->i_hw_irq); +} +#endif + int arm_setup_intr(const char *name, driver_filter_t *filt, driver_intr_t handler, void *arg, u_int hw_irq, enum intr_type flags, void **cookiep) @@ -339,6 +352,9 @@ arm_setup_intr(const char *name, driver_filter_t *filt, driver_intr_t handler, return (error); } + if ((flags & INTR_PERCPU) != 0) + intr->i_percpu = true; + error = intr_event_add_handler(intr->i_event, name, filt, handler, arg, intr_priority(flags), flags, cookiep); @@ -354,7 +370,7 @@ arm_setup_intr(const char *name, driver_filter_t *filt, driver_intr_t handler, intr->i_pol); } - PIC_UNMASK(root_pic, intr->i_hw_irq); + smp_rendezvous(NULL, unmask_irq, NULL, intr); } mtx_unlock(&intr_list_lock); } @@ -466,8 +482,14 @@ arm_unmask_ipi(u_int ipi) void arm_init_secondary(void) { + struct arm64_intr_entry *intr; PIC_INIT_SECONDARY(root_pic); + + SLIST_FOREACH(intr, &irq_slist_head, entries) { + if (intr->i_percpu) + PIC_UNMASK(root_pic, intr->i_hw_irq); + } } /* Sending IPI */ diff --git a/sys/sys/bus.h b/sys/sys/bus.h index 42d3a3f..37ef5b5 100644 --- a/sys/sys/bus.h +++ b/sys/sys/bus.h @@ -247,6 +247,7 @@ enum intr_type { INTR_EXCL = 256, /* exclusive interrupt */ INTR_MPSAFE = 512, /* this interrupt is SMP safe */ INTR_ENTROPY = 1024, /* this interrupt provides entropy */ + INTR_PERCPU = 2048, /* enabled on all cpus */ INTR_MD1 = 4096, /* flag reserved for MD use */ INTR_MD2 = 8192, /* flag reserved for MD use */ INTR_MD3 = 16384, /* flag reserved for MD use */ -- 2.4.6 --MP_/yGe585u0HCR6ngMWAQOHcw/-- From owner-freebsd-arch@freebsd.org Fri Jul 24 10:29:21 2015 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 A74479A9079 for ; Fri, 24 Jul 2015 10:29:21 +0000 (UTC) (envelope-from meloun@miracle.cz) Received: from mail.miracle.cz (mail.miracle.cz [193.84.128.19]) by mx1.freebsd.org (Postfix) with ESMTP id 69EF015D8 for ; Fri, 24 Jul 2015 10:29:21 +0000 (UTC) (envelope-from meloun@miracle.cz) Received: from [193.84.128.50] (meloun.ad.miracle.cz [193.84.128.50]) by mail.miracle.cz (Postfix) with ESMTPSA id 5ECB6ACABEB for ; Fri, 24 Jul 2015 12:22:33 +0200 (CEST) Message-ID: <55B211FA.3010700@miracle.cz> Date: Fri, 24 Jul 2015 12:22:50 +0200 From: Michal Meloun Organization: Miracle Group User-Agent: Mozilla/5.0 (Windows NT 5.1; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 MIME-Version: 1.0 To: freebsd-arch@freebsd.org Subject: Re: Interrupts on all CPUs References: <20150723182449.0ecc7ea9@bender> In-Reply-To: <20150723182449.0ecc7ea9@bender> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.4.3 (mail.miracle.cz); Fri, 24 Jul 2015 12:22:33 +0200 (CEST) X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 24 Jul 2015 10:29:21 -0000 > We have a problem on ARM where some interrupts need to be unmasked on > all CPUs in an SMP environment. The main place we need this is with the > timer driver as the hardware can send interrupts to the requested CPU. > > Until now we have hacked around this by unmasking the three interrupts > the timer hardware uses when we enable the non-boot CPU. Luckily these > interrupts values have been common across all SoCs, however there is no > requirement for this to be the case. > > To fix this I'm proposing adding a flag to bus_setup_intr to add > support for per-cpu interrupts. The architecture code is then expected > to perform whatever is needed to handle this. > > I have attached a proof of concept patch that adds support to arm64 for > this. When we setup an interrupt with the flag set it will record this > so when the non-boot CPUs are enabled they can unmask the interrupt. It > will then signal to any running CPUs to unmask the given interrupt. > > I would expect this could be extended to other architectures, however I > only know the arm and arm64 code, and am only able to test there. > > Andrew I don’t think that this patch solves the problem. In some case, we must initialize per core hardware before we can enable interrupt for it. This is for example, case of the qcom Krait per core devices. I'm thinking about something like that config_intrhook for registering “secondary_attach()” functions from each appropriate driver. The list can be executed as part of secondary_init(), in same order as registered. This, of course implies existence of specific SGI and PPI variant of bus_setup_interrupt() that have core ID as argument and allows multiple activation. Moreover, this approach removed necessary of direct calls for initialization of secondary cores in gic and timer drivers. What do you think? Michal