Date: Tue, 29 Apr 2014 18:07:56 +0000 From: "Wojciech A. Koszek" <wkoszek@freebsd.org> To: Thomas Skibo <ThomasSkibo@sbcglobal.net> Cc: freebsd-arm <freebsd-arm@freebsd.org> Subject: Re: SMP support for ZEDBOARD Message-ID: <20140429180756.GP91461@FreeBSD.org> In-Reply-To: <535EEB12.2050704@sbcglobal.net> References: <535EEB12.2050704@sbcglobal.net>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <sys/cdefs.h>
> +__FBSDID("$FreeBSD$");
You need \n here.
> +#include <sys/param.h>
> +#include <sys/systm.h>
> +#include <sys/bus.h>
> +#include <sys/lock.h>
> +#include <sys/mutex.h>
> +#include <sys/smp.h>
> +
> +#include <machine/smp.h>
> +#include <machine/fdt.h>
> +#include <machine/intr.h>
> +
> +#include <arm/xilinx/zy7_reg.h>
> +
> +#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?
--
Wojciech A. Koszek
wkoszek@FreeBSD.czest.pl
http://FreeBSD.czest.pl/~wkoszek/
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140429180756.GP91461>