[U-Boot] [PATCH v5 1/4] core support of arm64
FengHua
fenghua at phytium.com.cn
Wed Sep 11 16:13:03 CEST 2013
> -----原始邮件-----
> 发件人: "Scott Wood" <scottwood at freescale.com>
> 发送时间: 2013年9月10日 星期二
> 收件人: FengHua <fenghua at phytium.com.cn>
> 抄送: u-boot at lists.denx.de, trini at ti.com
> 主题: Re: Re: [U-Boot] [PATCH v5 1/4] core support of arm64
>
> On Sat, 2013-09-07 at 22:56 +0800, FengHua wrote:
> > Fisrt, thank scott for checking the patch.
> >
> > > -----原始邮件-----
> > > 发件人: "Scott Wood" <scottwood at freescale.com>
> > > 发送时间: 2013年9月7日 星期六
> > > 收件人: fenghua at phytium.com.cn
> > > 抄送: u-boot at lists.denx.de, trini at ti.com
> > > 主题: Re: [U-Boot] [PATCH v5 1/4] core support of arm64
> > >
> > > On Sat, 2013-08-24 at 09:06 +0800, fenghua at phytium.com.cn wrote:
> > > > From: David Feng <fenghua at phytium.com.cn>
> > > >
> > > > Signed-off-by: David Feng <fenghua at phytium.com.cn>
> > > > ---
> > > > Changeds for v4:
> > > > - Replace __ARMEB__ with __AARCH64EB__ in byteorder.h and unaligned.h,
> > > > gcc for aarch64 use __AARCH64EB__ and __AARCH64EL__ to identify endian.
> > > > - Some modification to README.armv8
> > > >
> > > > arch/arm/config.mk | 4 +
> > > > arch/arm/cpu/armv8/Makefile | 56 ++++++
> > > > arch/arm/cpu/armv8/cache.S | 145 +++++++++++++++
> > > > arch/arm/cpu/armv8/cache_v8.c | 291 +++++++++++++++++++++++++++++++
> > > > arch/arm/cpu/armv8/config.mk | 31 ++++
> > > > arch/arm/cpu/armv8/cpu.c | 68 ++++++++
> > > > arch/arm/cpu/armv8/crt0.S | 130 ++++++++++++++
> > > > arch/arm/cpu/armv8/exceptions.S | 182 +++++++++++++++++++
> > > > arch/arm/cpu/armv8/interrupts.c | 116 ++++++++++++
> > > > arch/arm/cpu/armv8/relocate.S | 71 ++++++++
> > > > arch/arm/cpu/armv8/start.S | 200 +++++++++++++++++++++
> > > > arch/arm/cpu/armv8/timer.c | 95 ++++++++++
> > > > arch/arm/cpu/armv8/tlb.S | 38 ++++
> > > > arch/arm/cpu/armv8/u-boot.lds | 83 +++++++++
> > >
> > > Subject says "arm64", files say "armv8".
> > >
> > > Some of these, such as relocate.S, probably are not going to be
> > > armv8-specific but just arm64-specific.
> > >
> > > > arch/arm/include/asm/arch-armv8/armv8.h | 44 +++++
> > > > arch/arm/include/asm/arch-armv8/gpio.h | 26 +++
> > > > arch/arm/include/asm/arch-armv8/mmu.h | 117 +++++++++++++
> > > > arch/arm/include/asm/byteorder.h | 12 ++
> > > > arch/arm/include/asm/config.h | 10 ++
> > > > arch/arm/include/asm/global_data.h | 6 +-
> > > > arch/arm/include/asm/io.h | 12 +-
> > > > arch/arm/include/asm/macro.h | 26 +++
> > > > arch/arm/include/asm/posix_types.h | 31 ++++
> > > > arch/arm/include/asm/proc-armv/ptrace.h | 38 ++++
> > > > arch/arm/include/asm/proc-armv/system.h | 58 +++++-
> > > > arch/arm/include/asm/types.h | 14 ++
> > > > arch/arm/include/asm/u-boot.h | 4 +
> > > > arch/arm/include/asm/unaligned.h | 14 ++
> > > > arch/arm/lib/Makefile | 8 +
> > > > arch/arm/lib/board.c | 18 ++
> > > > arch/arm/lib/bootm.c | 16 ++
> > > > common/image.c | 1 +
> > > > doc/README.armv8 | 14 ++
> > > > examples/standalone/stubs.c | 15 ++
> > > > include/image.h | 1 +
> > > > 35 files changed, 1987 insertions(+), 8 deletions(-)
> > > > create mode 100644 arch/arm/cpu/armv8/Makefile
> > > > create mode 100644 arch/arm/cpu/armv8/cache.S
> > > > create mode 100644 arch/arm/cpu/armv8/cache_v8.c
> > > > create mode 100644 arch/arm/cpu/armv8/config.mk
> > > > create mode 100644 arch/arm/cpu/armv8/cpu.c
> > > > create mode 100644 arch/arm/cpu/armv8/crt0.S
> > > > create mode 100644 arch/arm/cpu/armv8/exceptions.S
> > > > create mode 100644 arch/arm/cpu/armv8/interrupts.c
> > > > create mode 100644 arch/arm/cpu/armv8/relocate.S
> > > > create mode 100644 arch/arm/cpu/armv8/start.S
> > > > create mode 100644 arch/arm/cpu/armv8/timer.c
> > > > create mode 100644 arch/arm/cpu/armv8/tlb.S
> > > > create mode 100644 arch/arm/cpu/armv8/u-boot.lds
> > > > create mode 100644 arch/arm/include/asm/arch-armv8/armv8.h
> > > > create mode 100644 arch/arm/include/asm/arch-armv8/gpio.h
> > > > create mode 100644 arch/arm/include/asm/arch-armv8/mmu.h
> > > > create mode 100644 doc/README.armv8
> > > >
> > > > diff --git a/arch/arm/config.mk b/arch/arm/config.mk
> > > > index ce3903b..f1c6a7b 100644
> > > > --- a/arch/arm/config.mk
> > > > +++ b/arch/arm/config.mk
> > > > @@ -74,7 +74,9 @@ endif
> > > > endif
> > > >
> > > > # needed for relocation
> > > > +ifndef CONFIG_ARMV8
> > > > LDFLAGS_u-boot += -pie
> > > > +endif
> > >
> > > CONFIG_ARM64 (until we fix this, of course)
> > >
> > > > #
> > > > # FIXME: binutils versions < 2.22 have a bug in the assembler where
> > > > @@ -95,6 +97,8 @@ endif
> > > > endif
> > > >
> > > > # check that only R_ARM_RELATIVE relocations are generated
> > > > +ifndef CONFIG_ARMV8
> > > > ifneq ($(CONFIG_SPL_BUILD),y)
> > > > ALL-y += checkarmreloc
> > > > endif
> > > > +endif
> > >
> > > ARM64, though I've got a patch coming that will make the check work with
> > > arm64.
> > >
> > > Likewise elsewhere -- most if not all of the CONFIG_ARMV8 uses should be
> > > CONFIG_ARM64.
> > >
> >
> > Actually, the naming is so confusing. The directory's name is armv8,
> > but it only represents aarch64 here. So, whether CONFIG_ARMV8 or CONFIG_ARM64 should be used
> > is difficult to make decision.
>
> Files that are about 64-bit rather than armv8 should not go in the armv8
> directory. E.g. relocate should be arch/arm/lib/relocate64.S.
>
crt0.S and relocate.S both could be moved to /arm/lib/ and renamed.
Currently, I try to keep armv8 specific codes within armv8 directory
and do not mixed with original arm code. Absolutely we should did so when armv9 coming.
> > > > +/*
> > > > + * void __asm_flush_dcache_level(level)
> > > > + *
> > > > + * clean and invalidate one level cache.
> > > > + *
> > > > + * x0: cache level
> > > > + * x1~x9: clobbered
> > > > + */
> > > > +ENTRY(__asm_flush_dcache_level)
> > >
> > > Why the leading underscores?
> > >
> >
> > Is there any convention with underscores? I just know the underscore represent that the function is best not be called directly.
>
> Sometimes that's done when there are multiple versions of a function,
> but it's bad practice since you're in a reserved namespace (especially
> if you go and do the same thing in a normal userspace program). In this
> case there isn't even a non-underscore version, so it's just gratuitous.
>
> > > > +/*
> > > > + * Stub implementations for outer cache operations
> > > > + */
> > > > +void __v8_outer_cache_enable(void) {}
> > > > +void v8_outer_cache_enable(void)
> > > > + __attribute__((weak, alias("__v8_outer_cache_enable")));
> > > > +
> > > > +void __v8_outer_cache_disable(void) {}
> > > > +void v8_outer_cache_disable(void)
> > > > + __attribute__((weak, alias("__v8_outer_cache_disable")));
> > > > +
> > > > +void __v8_outer_cache_flush_all(void) {}
> > > > +void v8_outer_cache_flush_all(void)
> > > > + __attribute__((weak, alias("__v8_outer_cache_flush_all")));
> > > > +
> > > > +void __v8_outer_cache_inval_all(void) {}
> > > > +void v8_outer_cache_inval_all(void)
> > > > + __attribute__((weak, alias("__v8_outer_cache_inval_all")));
> > > > +
> > > > +void __v8_outer_cache_flush_range(u64 start, u64 end) {}
> > > > +void v8_outer_cache_flush_range(u64 start, u64 end)
> > > > + __attribute__((weak, alias("__v8_outer_cache_flush_range")));
> > > > +
> > > > +void __v8_outer_cache_inval_range(u64 start, u64 end) {}
> > > > +void v8_outer_cache_inval_range(u64 start, u64 end)
> > > > + __attribute__((weak, alias("__v8_outer_cache_inval_range")));
> > >
> > > What level do you anticipate these being overriden at? Individual
> > > chips? Why?
> > It's socs. I don't know how the armv8 processor would like to be. It's just derived from __v7_outer_cache*.
>
> Is this for flushing caches that are outside the core? On powerpc you
> could do that with architected instructions for a range flush -- is that
> not the case with ARM? Do the IC/DC instructions not operate on caches
> outside the core?
I noticed that cache-pl310.c implements v7_outer_* functions.
IC/DC instructions could maintain caches in cache hierarchy of armv8
whatever the cache is outside the core or inside the core.
But armv8 architecture reference manual(not totally completed) do not
require all caches exist in the cache hierarchy. Maybe some processors
have it's own cache level that is not maintained by IC/DC instructions.
who knows.
I keep this just because it existed in armv7.
Best regards,
David
>
> > > > diff --git a/arch/arm/cpu/armv8/config.mk b/arch/arm/cpu/armv8/config.mk
> > > > new file mode 100644
> > > > index 0000000..aae2170
> > > > --- /dev/null
> > > > +++ b/arch/arm/cpu/armv8/config.mk
> > > > @@ -0,0 +1,31 @@
> > > > +#
> > > > +# Copyright (c) 2013 FengHua <fenghua at phytium.com.cn>
> > > > +#
> > > > +# See file CREDITS for list of people who contributed to this
> > > > +# project.
> > > > +#
> > > > +# 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, write to the Free Software
> > > > +# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> > > > +# MA 02111-1307 USA
> > > > +#
> > > > +PLATFORM_RELFLAGS += -fno-common -ffixed-x18
> > >
> > > Is -ffixed-x18 necessary, or will GCC do the right thing when it sees
> > > that x18 has been declared as a register variable?
> > >
> > I am not sure and will check it.
>
> GCC complains without --fixed-x18 because it views it as a volatile
> register. The ABI says this about it: "The Platform Register. If the
> platform ABI needs a dedicated register, this is it. Otherwise a
> temporary register."
>
> > > > diff --git a/arch/arm/include/asm/macro.h b/arch/arm/include/asm/macro.h
> > > > index ff13f36..70be196 100644
> > > > --- a/arch/arm/include/asm/macro.h
> > > > +++ b/arch/arm/include/asm/macro.h
> > > > @@ -54,5 +54,31 @@
> > > > bcs 1b
> > > > .endm
> > > >
> > > > +#ifdef CONFIG_ARMV8
> > > > +/*
> > > > + * Register aliases.
> > > > + */
> > > > +lr .req x30 // link register
> > > > +
> > > > +/*
> > > > + * Stack pushing/popping (register pairs only).
> > > > + * Equivalent to store decrement before,
> > > > + * load increment after.
> > > > + */
> > > > +.macro push, xreg1, xreg2
> > > > + stp \xreg1, \xreg2, [sp, #-16]!
> > > > +.endm
> > > > +
> > > > +.macro pop, xreg1, xreg2
> > > > + ldp \xreg1, \xreg2, [sp], #16
> > > > +.endm
> > >
> > > More uncredited and license-incompatible Linux copying (the macros
> > > themselves might not be copyrightable, but the comments are...).
> > >
> > I will modify the comments.
>
> Please be aware that modifying the comments is generally not an
> acceptable alternative to rewriting from scratch. The non-comment
> portion of the above quote might not be regarded as copyrightable, but
> we're not lawyers and shouldn't rely on such things if we can avoid it.
>
> > > > diff --git a/arch/arm/include/asm/posix_types.h b/arch/arm/include/asm/posix_types.h
> > > > index c412486..0da38cb 100644
> > > > --- a/arch/arm/include/asm/posix_types.h
> > > > +++ b/arch/arm/include/asm/posix_types.h
> > > > @@ -19,6 +19,8 @@
> > > > * assume GCC is being used.
> > > > */
> > > >
> > > > +#ifndef CONFIG_ARMV8
> > >
> > > You need to include config.h first. For example, the warnings in
> > > lib/hashtable.c are due to picking up the 32-bit definition of
> > > __kernel_size_t.
> >
> > Maybe, it is better that config.h is included in hashtable.c
>
> No. If you use a config symbol in this file, then this file should
> include config.h. Otherwise we'll silently break on the next file that
> has a similar problem.
>
> -Scott
>
>
>
More information about the U-Boot
mailing list