From owner-freebsd-arm@FreeBSD.ORG Tue Apr 29 19:05:17 2014 Return-Path: Delivered-To: freebsd-arm@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id A4609C43; Tue, 29 Apr 2014 19:05:17 +0000 (UTC) Received: from mho-01-ewr.mailhop.org (mho-03-ewr.mailhop.org [204.13.248.66]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 6581ADC5; Tue, 29 Apr 2014 19:05:16 +0000 (UTC) Received: from c-24-8-230-52.hsd1.co.comcast.net ([24.8.230.52] helo=damnhippie.dyndns.org) by mho-01-ewr.mailhop.org with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.72) (envelope-from ) id 1WfDL9-000BmG-Jm; Tue, 29 Apr 2014 19:05:15 +0000 Received: from [172.22.42.240] (revolution.hippie.lan [172.22.42.240]) by damnhippie.dyndns.org (8.14.3/8.14.3) with ESMTP id s3TJ5D5r016623; Tue, 29 Apr 2014 13:05:13 -0600 (MDT) (envelope-from ian@FreeBSD.org) X-Mail-Handler: Dyn Standard SMTP by Dyn X-Originating-IP: 24.8.230.52 X-Report-Abuse-To: abuse@dyndns.com (see http://www.dyndns.com/services/sendlabs/outbound_abuse.html for abuse reporting information) X-MHO-User: U2FsdGVkX1/oJxB89zhySry6Xr4wy8ei Subject: Re: SMP support for ZEDBOARD From: Ian Lepore To: "Wojciech A. Koszek" In-Reply-To: <20140429180756.GP91461@FreeBSD.org> References: <535EEB12.2050704@sbcglobal.net> <20140429180756.GP91461@FreeBSD.org> Content-Type: text/plain; charset="us-ascii" Date: Tue, 29 Apr 2014 13:05:13 -0600 Message-ID: <1398798313.22079.31.camel@revolution.hippie.lan> Mime-Version: 1.0 X-Mailer: Evolution 2.32.1 FreeBSD GNOME Team Port Content-Transfer-Encoding: 7bit Cc: freebsd-arm , Thomas Skibo X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "Porting FreeBSD to ARM processors." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 29 Apr 2014 19:05:17 -0000 On Tue, 2014-04-29 at 18:07 +0000, Wojciech A. Koszek wrote: > On Mon, Apr 28, 2014 at 04:58:10PM -0700, Thomas Skibo wrote: > > > > Hi, All. > > > > I continue to test FreeBSD on the Zedboard and SMP seems very solid. Can > > I get someone to commit SMP support to head (diffs attached)? > > > > I also have some bug-fixes too, mainly for the ethernet driver. Should > > I just file bugs and provide the patches? > > > > Thomas > > This certainly looks good. Small style nit below. > > > Index: sys/arm/xilinx/zy7_mp.c > > =================================================================== > > --- /dev/null 2014-04-09 08:33:00.000000000 -0700 > > +++ sys/arm/xilinx/zy7_mp.c 2014-04-05 11:28:29.000000000 -0700 > > @@ -0,0 +1,98 @@ > > +/*- > > + * Copyright (c) 2013 Thomas Skibo. All rights reserved. > > + * > > + * Redistribution and use in source and binary forms, with or without > > + * modification, are permitted provided that the following conditions > > + * are met: > > + * 1. Redistributions of source code must retain the above copyright > > + * notice, this list of conditions and the following disclaimer. > > + * 2. Redistributions in binary form must reproduce the above copyright > > + * notice, this list of conditions and the following disclaimer in the > > + * documentation and/or other materials provided with the distribution. > > + * > > + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR > > + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES > > + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. > > + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, > > + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT > > + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, > > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY > > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF > > + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > > + */ > > + > > +#include > > +__FBSDID("$FreeBSD$"); > > You need \n here. > > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > +#include > > +#include > > + > > +#include > > + > > +#define ZYNQ7_CPU1_ENTRY 0xfffffff0 > > + > > +void > > +platform_mp_init_secondary(void) > > +{ > > + > > + gic_init_secondary(); > > +} > > + > > +void > > +platform_mp_setmaxid(void) > > +{ > > + > > + mp_maxid = 1; > > You have spaces here instead of TAB for alignment. > > > +} > > + > > +int > > +platform_mp_probe(void) > > +{ > > + > > + mp_ncpus = 2; > > Why does ARM port of FreeBSD need that? Why isn't number of CPUs read from > the SCU register? This method sounds redundant (at least across ARM Cortex > family) > > > + return (1); > > +} > > + > > +void > > +platform_mp_start_ap(void) > > +{ > > + bus_space_handle_t ocm_handle; > > + > > + /* Map in magic location to give entry address to CPU1. */ > > + if (bus_space_map(fdtbus_bs_tag, ZYNQ7_CPU1_ENTRY, 4, > > + 0, &ocm_handle) != 0) > > + panic("platform_mp_start_ap: Couldn't map OCM\n"); > > + > > + /* Write start address for CPU1. */ > > + bus_space_write_4(fdtbus_bs_tag, ocm_handle, 0, > > + pmap_kextract((vm_offset_t)mpentry)); > > + > > + /* The SCU is enabled by the BOOTROM but I think the second CPU > > You probably need to start comments with: > > /* > * ... > > (missing \n) > > > + * doesn't turn on filtering until after the wake-up below. > > + * I think that's why things don't work if I don't put these > > + * cache ops here. Also, the magic location, 0xfffffff0, isn't > > + * in the SCU's filtering range so it needs a write-back too. > > + */ > > + cpu_idcache_wbinv_all(); > > + cpu_l2cache_wbinv_all(); > > + > > + /* Wake up CPU1. */ > > + armv7_sev(); > > + > > + bus_space_unmap(fdtbus_bs_tag, ocm_handle, 4); > > +} > > + > > +void > > +platform_ipi_send(cpuset_t cpus, u_int ipi) > > +{ > > + > > + pic_ipi_send(cpus, ipi); > > +} > > Index: sys/arm/conf/ZEDBOARD > > =================================================================== > > --- sys/arm/conf/ZEDBOARD (revision 264766) > > +++ sys/arm/conf/ZEDBOARD (working copy) > > @@ -56,6 +56,7 @@ > > options _KPOSIX_PRIORITY_SCHEDULING # Posix P1003_1B real-time extensions > > options FREEBSD_BOOT_LOADER > > options VFP # vfp/neon > > +options SMP # Symmetric MultiProcessor Kernel > > I think we have \t\t instead of just \t here. > > > > > # Debugging > > makeoptions DEBUG=-g > > Index: sys/arm/xilinx/files.zynq7 > > =================================================================== > > --- sys/arm/xilinx/files.zynq7 (revision 264766) > > +++ sys/arm/xilinx/files.zynq7 (working copy) > > @@ -21,6 +21,7 @@ > > arm/xilinx/zy7_bus_space.c standard > > arm/xilinx/zy7_slcr.c standard > > arm/xilinx/zy7_devcfg.c standard > > +arm/xilinx/zy7_mp.c optional smp > > > > dev/cadence/if_cgem.c optional if_cgem > > dev/sdhci/sdhci_fdt.c optional sdhci > > Index: sys/arm/xilinx/std.zynq7 > > =================================================================== > > --- sys/arm/xilinx/std.zynq7 (revision 264766) > > +++ sys/arm/xilinx/std.zynq7 (working copy) > > @@ -20,3 +20,5 @@ > > > > options ARM_L2_PIPT > > > > +options IPI_IRQ_START=0 > > +options IPI_IRQ_END=15 > > > > Run with older U-Boot, does it just work? > I tidied up the whitespace nits you noted when I did the commit. About the platform_mp_probe() and multicore startup code in general, I've been thinking we should have generic armv7/mpcore code for doing almost everything except the actual launching of the AP cores, which does tend to be wildly different amongst the various SoCs. -- Ian