| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <1227082206.24481.44.camel@dyang>
Date: Wed, 19 Nov 2008 16:10:06 +0800
From: gyang <graf.yang@...log.com>
To: Bryan Wu <cooloney@...nel.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
torvalds@...ux-foundation.org, mingo@...e.hu,
linux-kernel@...r.kernel.org,
Mike Frysinger <vapier.adi@...il.com>,
linux-arch@...r.kernel.org
Subject: Re: [PATCH 1/5] Blackfin arch: SMP supporting patchset: BF561
related code
在 2008-11-19三的 15:39 +0800，Bryan Wu写道：
> On Wed, Nov 19, 2008 at 2:56 PM, Andrew Morton
> <akpm@...ux-foundation.org> wrote:
> > On Tue, 18 Nov 2008 17:05:04 +0800 Bryan Wu <cooloney@...nel.org> wrote:
> >
> >> From: Graf Yang <graf.yang@...log.com>
> >>
> >> Blackfin dual core BF561 processor can support SMP like features.
> >> https://docs.blackfin.uclinux.org/doku.php?id=linux-kernel:smp-like
> >>
> >> In this patch, we provide SMP extend to BF561 kernel code
> >>
> >>
> >> ...
> >>
> >> --- a/arch/blackfin/mach-bf561/include/mach/mem_map.h
> >> +++ b/arch/blackfin/mach-bf561/include/mach/mem_map.h
> >> @@ -85,4 +85,124 @@
> >> #define L1_SCRATCH_START COREA_L1_SCRATCH_START
> >> #define L1_SCRATCH_LENGTH 0x1000
> >>
> >> +#ifndef __ASSEMBLY__
> >> +
> >> +#ifdef CONFIG_SMP
> >> +
> >> +#define get_l1_scratch_start_cpu(cpu) \
> >> + ({ unsigned long __addr; \
> >> + __addr = (cpu) ? COREB_L1_SCRATCH_START : COREA_L1_SCRATCH_START;\
> >> + __addr; })
> >> +
> >> +#define get_l1_code_start_cpu(cpu) \
> >> + ({ unsigned long __addr; \
> >> + __addr = (cpu) ? COREB_L1_CODE_START : COREA_L1_CODE_START; \
> >> + __addr; })
> >> +
> >> +#define get_l1_data_a_start_cpu(cpu) \
> >> + ({ unsigned long __addr; \
> >> + __addr = (cpu) ? COREB_L1_DATA_A_START : COREA_L1_DATA_A_START;\
> >> + __addr; })
> >> +
> >> +#define get_l1_data_b_start_cpu(cpu) \
> >> + ({ unsigned long __addr; \
> >> + __addr = (cpu) ? COREB_L1_DATA_B_START : COREA_L1_DATA_B_START;\
> >> + __addr; })
> >> +
> >> +#define get_l1_scratch_start() get_l1_scratch_start_cpu(blackfin_core_id())
> >> +#define get_l1_code_start() get_l1_code_start_cpu(blackfin_core_id())
> >> +#define get_l1_data_a_start() get_l1_data_a_start_cpu(blackfin_core_id())
> >> +#define get_l1_data_b_start() get_l1_data_b_start_cpu(blackfin_core_id())
> >> +
> >> +#else /* !CONFIG_SMP */
> >> +#define get_l1_scratch_start_cpu(cpu) L1_SCRATCH_START
> >> +#define get_l1_code_start_cpu(cpu) L1_CODE_START
> >> +#define get_l1_data_a_start_cpu(cpu) L1_DATA_A_START
> >> +#define get_l1_data_b_start_cpu(cpu) L1_DATA_B_START
> >> +#define get_l1_scratch_start() L1_SCRATCH_START
> >> +#define get_l1_code_start() L1_CODE_START
> >> +#define get_l1_data_a_start() L1_DATA_A_START
> >> +#define get_l1_data_b_start() L1_DATA_B_START
> >> +#endif /* !CONFIG_SMP */
> >
> > grumble. These didn't need to be implemented as macros and hence
> > shouldn't have been.
> >
> > Example:
> >
> > int cpu = smp_processor_id();
> > get_l1_scratch_start_cpu(cpu);
> >
> > that code should generate unused variable warnings on CONFIG_SMP=n. If
> > it doesn't, you got lucky, because it _should_.
> >
> > Also
> >
> > int cpu = smp_processor_id();
> > get_l1_scratch_start_cpu(pcu);
> >
> > will happily compile and run with CONFIG_SMP=n.
> >
> >
> > macros=bad,bad,bad.
> >
>
> Yes, I also prefer inline functions rather than macros here.
> Right, Graf?
OK!
>
> >>
> >> ...
> >>
> >> --- /dev/null
> >> +++ b/arch/blackfin/mach-bf561/smp.c
> >> @@ -0,0 +1,182 @@
> >> +/*
> >> + * File: arch/blackfin/mach-bf561/smp.c
> >> + * Author: Philippe Gerum <rpm@...omai.org>
> >> + *
> >> + * Copyright 2007 Analog Devices Inc.
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License as published by
> >> + * the Free Software Foundation; either version 2 of the License, or
> >> + * (at your option) any later version.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >> + * GNU General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU General Public License
> >> + * along with this program; if not, see the file COPYING, or write
> >> + * to the Free Software Foundation, Inc.,
> >> + * 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> >> + */
> >> +
> >> +#include <linux/init.h>
> >> +#include <linux/kernel.h>
> >> +#include <linux/sched.h>
> >> +#include <linux/delay.h>
> >> +#include <asm/smp.h>
> >> +#include <asm/dma.h>
> >> +
> >> +#define COREB_SRAM_BASE 0xff600000
> >> +#define COREB_SRAM_SIZE 0x4000
> >> +
> >> +extern char coreb_trampoline_start, coreb_trampoline_end;
> >
> > OK, these are defined in .S and we do often put declarations for such
> > things in .c rather than in .h. But I think it's better to put them in
> > .h anyway, to avoid possibly duplicated declarations in the future.
> >
>
> Oh, I suggested Graf to run checkpatch.pl to find some issues before I
> sent out this patch.
> Should this issues be catched by checkpatch.pl?
OK, I will remove them.
>
>
> >> +static DEFINE_SPINLOCK(boot_lock);
> >> +
> >> +static cpumask_t cpu_callin_map;
> >> +
> >>
> >> ...
> >>
> >> +void __cpuinit platform_secondary_init(unsigned int cpu)
> >> +{
> >> + local_irq_disable();
> >> +
> >> + /* Clone setup for peripheral interrupt sources from CoreA. */
> >> + bfin_write_SICB_IMASK0(bfin_read_SICA_IMASK0());
> >> + bfin_write_SICB_IMASK1(bfin_read_SICA_IMASK1());
> >> + SSYNC();
> >> +
> >> + /* Clone setup for IARs from CoreA. */
> >> + bfin_write_SICB_IAR0(bfin_read_SICA_IAR0());
> >> + bfin_write_SICB_IAR1(bfin_read_SICA_IAR1());
> >> + bfin_write_SICB_IAR2(bfin_read_SICA_IAR2());
> >> + bfin_write_SICB_IAR3(bfin_read_SICA_IAR3());
> >> + bfin_write_SICB_IAR4(bfin_read_SICA_IAR4());
> >> + bfin_write_SICB_IAR5(bfin_read_SICA_IAR5());
> >> + bfin_write_SICB_IAR6(bfin_read_SICA_IAR6());
> >> + bfin_write_SICB_IAR7(bfin_read_SICA_IAR7());
> >> + SSYNC();
> >> +
> >> + local_irq_enable();
> >> +
> >> + /* Calibrate loops per jiffy value. */
> >> + calibrate_delay();
> >> +
> >> + /* Store CPU-private information to the cpu_data array. */
> >> + bfin_setup_cpudata(cpu);
> >> +
> >> + /* We are done with local CPU inits, unblock the boot CPU. */
> >> + cpu_set(cpu, cpu_callin_map);
> >> + spin_lock(&boot_lock);
> >> + spin_unlock(&boot_lock);
> >
> > Is this spin_lock()+spin_unlock() supposed to block until the secondary
> > CPU is running? If so, I don't think it works.
> >
>
> We can remove these 2 line spin_lock+spin_unlock and it also works.
> But maybe we will add some operation between spin_lock and spin_unlock
> here in the future,
> we'd like to keep them.
>
> P.S. also forward this patch to linux-arch
>
> Thanks
> -Bryan
>
> >> +}
> >> +
> >>
> >> ...
> >>
> >
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists