Re: [U-Boot] [RESEND PATCH v14 07/10] arm64: core support

2013-11-11 Thread Simon Glass
Hi David,

On Mon, Oct 14, 2013 at 9:34 PM, feng...@phytium.com.cn wrote:

 From: David Feng feng...@phytium.com.cn

 Relocation code based on a patch by Scott Wood, which is:
 Signed-off-by: Scott Wood scottw...@freescale.com

 Signed-off-by: David Feng feng...@phytium.com.cn
 ---
  arch/arm/config.mk  |3 +-
  arch/arm/cpu/armv8/Makefile |   38 +
  arch/arm/cpu/armv8/cache.S  |  130 +
  arch/arm/cpu/armv8/cache_v8.c   |  218
 
  arch/arm/cpu/armv8/config.mk|   15 ++
  arch/arm/cpu/armv8/cpu.c|   67 +
  arch/arm/cpu/armv8/exceptions.S |  112 +++
  arch/arm/cpu/armv8/start.S  |  234
 +++
  arch/arm/cpu/armv8/timer.c  |   80 +++
  arch/arm/cpu/armv8/tlb.S|   30 
  arch/arm/cpu/armv8/u-boot.lds   |   89 
  arch/arm/include/asm/armv8/mmu.h|  110 +++
  arch/arm/include/asm/byteorder.h|   12 ++
  arch/arm/include/asm/cache.h|5 +
  arch/arm/include/asm/config.h   |6 +
  arch/arm/include/asm/global_data.h  |6 +-
  arch/arm/include/asm/io.h   |   15 +-
  arch/arm/include/asm/macro.h|   36 +
  arch/arm/include/asm/posix_types.h  |   10 ++
  arch/arm/include/asm/proc-armv/ptrace.h |   21 +++
  arch/arm/include/asm/proc-armv/system.h |   59 +++-
  arch/arm/include/asm/system.h   |   77 ++
  arch/arm/include/asm/types.h|4 +
  arch/arm/include/asm/u-boot.h   |4 +
  arch/arm/include/asm/unaligned.h|2 +-
  arch/arm/lib/Makefile   |   14 ++
  arch/arm/lib/board.c|7 +-
  arch/arm/lib/bootm.c|   16 +++
  arch/arm/lib/crt0_64.S  |  113 +++
  arch/arm/lib/interrupts_64.c|  120 
  arch/arm/lib/relocate_64.S  |   58 
  common/image.c  |1 +
  doc/README.arm64|   45 ++
  examples/standalone/stubs.c |   15 ++
  include/image.h |1 +
  35 files changed, 1762 insertions(+), 11 deletions(-)


I think this series needs to be rebased to mainline now. I also found that
it did not build for armv7 past this commit - if it helps I did a bit of
tweaking and pushed a branch to 'armv8' at u-boot-x86.git.

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [RESEND PATCH v14 07/10] arm64: core support

2013-11-08 Thread Scott Wood
On Fri, 2013-11-08 at 13:49 +0800, FengHua wrote:
  -原始邮件-
  发件人: Scott Wood scottw...@freescale.com
  发送时间: 2013-11-08 08:44:34 (星期五)
  收件人: feng...@phytium.com.cn
  抄送: u-boot@lists.denx.de, tr...@ti.com, albert.u.b...@aribaud.net
  主题: Re: [RESEND PATCH v14 07/10] arm64: core support
  
  On Tue, 2013-10-15 at 11:34 +0800, feng...@phytium.com.cn wrote:
   +/*
   + * void __asm_flush_dcache_level(level)
   + *
   + * clean and invalidate one level cache.
   + *
   + * x0: cache level
   + * x1~x9: clobbered
   + */
   +ENTRY(__asm_flush_dcache_level)
   + lsl x1, x0, #1
   + msr csselr_el1, x1  /* select cache level */
   + isb /* isb to sych the new cssr  csidr */
   + mrs x6, ccsidr_el1  /* read the new ccsidr */
   + and x2, x6, #7  /* x2 - length of the cache lines */
   + add x2, x2, #4  /* add 4 (line length offset) */
   + mov x3, #0x3ff
   + and x3, x3, x6, lsr #3  /* x3 - maximum number of way size */
   + clz w5, w3  /* bit position of way size */
  
  You should round up (so add w3, w3, w3; sub w3, w3, #1 before clz),
  since the architecture allows non-power-of-2 values for #sets/#ways.
  
 x3 is the way size after and x3, x3, x6, lsr #3.
 clz  w5, w3 is used to caculate the number of heading zero  of way size.
 The result will be used to generate parameter of dc cisw.
 we don't need to calculate the power-of-2 values for #ways.

According to the manual, the number of bits for the way field is
log2(associativity) rounded up to the nearest integer.  Counting the
leading zeroes does not round up -- you'll get the same number of
leading zeroes for e.g. 0x17 as for 0x10.

  BTW, I see some very similar comments, register usage, and code
  structure in the Linux code.  Are you *sure* this code wasn't derived
  from it (or some other common source)?  Do we need to start from
  scratch, if we can't trust that you're identifying all the code that you
  didn't write yourself?  You were asked several times to do so.
  
 The reference code is from ARMv8 ARM.

That still counts as code that came from somewhere else that needs to be
attributed.  What is the license on that code in the ARMv8 ARM?

Plus, I see some other similarities to the Linux code that aren't found
in the ARMv8 ARM code (e.g. add 4 (line length offset), create
working copy of [max] way size...).

 Cache.S of Linux-aarch64 is identical with it.
 Maybe the maintainer of Linux-aarch64 is just the writer of ARMv8 ARM.

There is an ARM copyright on that file in Linux...

 It's hard to write different code to do hardware specific initialization.

I'm not necessarily asking for different code, just attribution and
license compliance.

   + /* load TTBR0 */
   + el = curent_el();
   + if (el == 1)
   + asm volatile(msr ttbr0_el1, %0
   +  : : r (gd-arch.tlb_addr) : memory);
   + else if (el == 2)
   + asm volatile(msr ttbr0_el2, %0
   +  : : r (gd-arch.tlb_addr) : memory);
   + else
   + panic(Not Supported Exception Level);
  
  Do we really need to support running in either el1 or el2 at runtime,
  throughout U-Boot?  If Linux is started in el2, it enters el1 after
  setting up exception vectors to get control back if it needs to.  Can't
  we do the same (and go back to el2 if available immediately before
  entering an OS)?  Assuming we don't want to just set the expected
  exception level at compile time.
  
 I am preparing to ask for some advice about boot process.
 My original idea is to providing more choice for users. user could choose
 to run u-boot at el1 or el2.
 But now, locally I have changed the boot process. U-boot will be running
 at the highest exception level processor supportted (EL3/EL2/EL1)

By highest you mean closest to EL3?  That seems the opposite direction
of what would simplify things...

 I am not sure whether u-boot is ever used to provide run-time service before.

Only to standalone programs, not after booting an OS with bootm.

 There are many works to do if we want it be that.

We don't. :-)

-Scott



___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [RESEND PATCH v14 07/10] arm64: core support

2013-11-07 Thread York Sun
On 10/14/2013 08:34 PM, feng...@phytium.com.cn wrote:
 From: David Feng feng...@phytium.com.cn
 
 Relocation code based on a patch by Scott Wood, which is:
 Signed-off-by: Scott Wood scottw...@freescale.com
 
 Signed-off-by: David Feng feng...@phytium.com.cn
 ---
  arch/arm/config.mk  |3 +-
  arch/arm/cpu/armv8/Makefile |   38 +
  arch/arm/cpu/armv8/cache.S  |  130 +

snip

 diff --git a/arch/arm/cpu/armv8/cache.S b/arch/arm/cpu/armv8/cache.S
 new file mode 100644
 index 000..419f169
 --- /dev/null
 +++ b/arch/arm/cpu/armv8/cache.S
 @@ -0,0 +1,130 @@
 +/*
 + * (C) Copyright 2013
 + * David Feng feng...@phytium.com.cn
 + *
 + * SPDX-License-Identifier:  GPL-2.0+
 + */
 +
 +#include asm-offsets.h
 +#include config.h
 +#include version.h
 +#include asm/macro.h
 +#include linux/linkage.h
 +
 +/*
 + * void __asm_flush_dcache_level(level)
 + *
 + * clean and invalidate one level cache.
 + *
 + * x0: cache level
 + * x1~x9: clobbered
 + */
 +ENTRY(__asm_flush_dcache_level)
 + lsl x1, x0, #1
 + msr csselr_el1, x1  /* select cache level */
 + isb /* isb to sych the new cssr  csidr */
 + mrs x6, ccsidr_el1  /* read the new ccsidr */
 + and x2, x6, #7  /* x2 - length of the cache lines */
 + add x2, x2, #4  /* add 4 (line length offset) */
 + mov x3, #0x3ff
 + and x3, x3, x6, lsr #3  /* x3 - maximum number of way size */
 + clz w5, w3  /* bit position of way size */
 + mov x4, #0x7fff
 + and x4, x4, x1, lsr #13 /* x4 - max number of the set size */

Shouldn't this x1 be x6?


 + /* x1 - cache level  1 */
 + /* x2 - line length offset */
 + /* x3 - number of cache ways */
 + /* x4 - number of cache sets */
 + /* x5 - bit position of way size */
 +
 +loop_set:
 + mov x6, x3  /* create working copy of way size */
 +loop_way:
 + lsl x7, x6, x5
 + orr x9, x0, x7  /* map way and level to cisw value */

Shouldn't this x0 be x1?

 + lsl x7, x4, x2
 + orr x9, x9, x7  /* map set number to cisw value */
 + dc  cisw, x9/* clean  invalidate by set/way */
 + subsx6, x6, #1  /* decrement the way */
 + b.geloop_way
 + subsx4, x4, #1  /* decrement the set */
 + b.geloop_set
 +
 + ret
 +ENDPROC(__asm_flush_dcache_level)
 +

I haven't been able to verify this change. It simply takes too long to
finish on emulator.

York




___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [RESEND PATCH v14 07/10] arm64: core support

2013-11-07 Thread Scott Wood
On Tue, 2013-10-15 at 11:34 +0800, feng...@phytium.com.cn wrote:
 +/*
 + * void __asm_flush_dcache_level(level)
 + *
 + * clean and invalidate one level cache.
 + *
 + * x0: cache level
 + * x1~x9: clobbered
 + */
 +ENTRY(__asm_flush_dcache_level)
 + lsl x1, x0, #1
 + msr csselr_el1, x1  /* select cache level */
 + isb /* isb to sych the new cssr  csidr */
 + mrs x6, ccsidr_el1  /* read the new ccsidr */
 + and x2, x6, #7  /* x2 - length of the cache lines */
 + add x2, x2, #4  /* add 4 (line length offset) */
 + mov x3, #0x3ff
 + and x3, x3, x6, lsr #3  /* x3 - maximum number of way size */
 + clz w5, w3  /* bit position of way size */

You should round up (so add w3, w3, w3; sub w3, w3, #1 before clz),
since the architecture allows non-power-of-2 values for #sets/#ways.

Also s/way size/#ways/ and s/set size/#sets/

When I see set size I think of the number of ways times the size of a
cache line, not the number of sets.

BTW, I see some very similar comments, register usage, and code
structure in the Linux code.  Are you *sure* this code wasn't derived
from it (or some other common source)?  Do we need to start from
scratch, if we can't trust that you're identifying all the code that you
didn't write yourself?  You were asked several times to do so.

 +loop_level:
 + lsl x1, x0, #1
 + add x1, x1, x0  /* x0 - 3x cache level */

Comment says x0 gets 3x cache level, but the value is placed in x1.

 + sub x3, x2, #1
 + bic x0, x0, x3
 +1:  dc   civac, x0   /* clean  invalidate D/unified line */
 + add x0, x0, x2

Whitespace

 + /* load TTBR0 */
 + el = curent_el();
 + if (el == 1)
 + asm volatile(msr ttbr0_el1, %0
 +  : : r (gd-arch.tlb_addr) : memory);
 + else if (el == 2)
 + asm volatile(msr ttbr0_el2, %0
 +  : : r (gd-arch.tlb_addr) : memory);
 + else
 + panic(Not Supported Exception Level);

Do we really need to support running in either el1 or el2 at runtime,
throughout U-Boot?  If Linux is started in el2, it enters el1 after
setting up exception vectors to get control back if it needs to.  Can't
we do the same (and go back to el2 if available immediately before
entering an OS)?  Assuming we don't want to just set the expected
exception level at compile time.

-Scott



___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [RESEND PATCH v14 07/10] arm64: core support

2013-11-07 Thread york sun


 
 On Nov 7, 2013, at 9:03 PM, FengHua wrote:
 
 
 I have verified these two changes on Foudation Model. It's ok, very fast.
 
 

I have verified this change on emulator, too. I have to enable i-cache first to 
speed it up.

York



___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [RESEND PATCH v14 07/10] arm64: core support

2013-10-14 Thread fenghua
From: David Feng feng...@phytium.com.cn

Relocation code based on a patch by Scott Wood, which is:
Signed-off-by: Scott Wood scottw...@freescale.com

Signed-off-by: David Feng feng...@phytium.com.cn
---
 arch/arm/config.mk  |3 +-
 arch/arm/cpu/armv8/Makefile |   38 +
 arch/arm/cpu/armv8/cache.S  |  130 +
 arch/arm/cpu/armv8/cache_v8.c   |  218 
 arch/arm/cpu/armv8/config.mk|   15 ++
 arch/arm/cpu/armv8/cpu.c|   67 +
 arch/arm/cpu/armv8/exceptions.S |  112 +++
 arch/arm/cpu/armv8/start.S  |  234 +++
 arch/arm/cpu/armv8/timer.c  |   80 +++
 arch/arm/cpu/armv8/tlb.S|   30 
 arch/arm/cpu/armv8/u-boot.lds   |   89 
 arch/arm/include/asm/armv8/mmu.h|  110 +++
 arch/arm/include/asm/byteorder.h|   12 ++
 arch/arm/include/asm/cache.h|5 +
 arch/arm/include/asm/config.h   |6 +
 arch/arm/include/asm/global_data.h  |6 +-
 arch/arm/include/asm/io.h   |   15 +-
 arch/arm/include/asm/macro.h|   36 +
 arch/arm/include/asm/posix_types.h  |   10 ++
 arch/arm/include/asm/proc-armv/ptrace.h |   21 +++
 arch/arm/include/asm/proc-armv/system.h |   59 +++-
 arch/arm/include/asm/system.h   |   77 ++
 arch/arm/include/asm/types.h|4 +
 arch/arm/include/asm/u-boot.h   |4 +
 arch/arm/include/asm/unaligned.h|2 +-
 arch/arm/lib/Makefile   |   14 ++
 arch/arm/lib/board.c|7 +-
 arch/arm/lib/bootm.c|   16 +++
 arch/arm/lib/crt0_64.S  |  113 +++
 arch/arm/lib/interrupts_64.c|  120 
 arch/arm/lib/relocate_64.S  |   58 
 common/image.c  |1 +
 doc/README.arm64|   45 ++
 examples/standalone/stubs.c |   15 ++
 include/image.h |1 +
 35 files changed, 1762 insertions(+), 11 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/exceptions.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/armv8/mmu.h
 create mode 100644 arch/arm/lib/crt0_64.S
 create mode 100644 arch/arm/lib/interrupts_64.c
 create mode 100644 arch/arm/lib/relocate_64.S
 create mode 100644 doc/README.arm64

diff --git a/arch/arm/config.mk b/arch/arm/config.mk
index d0cf43f..a259193 100644
--- a/arch/arm/config.mk
+++ b/arch/arm/config.mk
@@ -17,7 +17,8 @@ endif
 
 LDFLAGS_FINAL += --gc-sections
 PLATFORM_RELFLAGS += -ffunction-sections -fdata-sections \
- -fno-common -ffixed-r9 -msoft-float
+ -fno-common -ffixed-r9
+PLATFORM_RELFLAGS += $(call cc-option, -msoft-float)
 
 # Support generic board on ARM
 __HAVE_ARCH_GENERIC_BOARD := y
diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile
new file mode 100644
index 000..b216f27
--- /dev/null
+++ b/arch/arm/cpu/armv8/Makefile
@@ -0,0 +1,38 @@
+#
+# (C) Copyright 2000-2003
+# Wolfgang Denk, DENX Software Engineering, w...@denx.de.
+#
+# SPDX-License-Identifier: GPL-2.0+
+#
+
+include $(TOPDIR)/config.mk
+
+LIB= $(obj)lib$(CPU).o
+
+START  := start.o
+
+COBJS  += cpu.o
+COBJS  += timer.o
+COBJS  += cache_v8.o
+
+SOBJS  += exceptions.o
+SOBJS  += cache.o
+SOBJS  += tlb.o
+
+SRCS   := $(START:.o=.S) $(COBJS:.o=.c)
+OBJS   := $(addprefix $(obj),$(COBJS) $(SOBJS))
+START  := $(addprefix $(obj),$(START))
+
+all:   $(obj).depend $(START) $(LIB)
+
+$(LIB):$(OBJS)
+   $(call cmd_link_o_target, $(OBJS))
+
+#
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#
diff --git a/arch/arm/cpu/armv8/cache.S b/arch/arm/cpu/armv8/cache.S
new file mode 100644
index 000..419f169
--- /dev/null
+++ b/arch/arm/cpu/armv8/cache.S
@@ -0,0 +1,130 @@
+/*
+ * (C) Copyright 2013
+ * David Feng feng...@phytium.com.cn
+ *
+ * SPDX-License-Identifier:GPL-2.0+
+ */
+
+#include asm-offsets.h
+#include config.h
+#include version.h
+#include asm/macro.h
+#include linux/linkage.h
+
+/*
+ * void __asm_flush_dcache_level(level)
+ *
+ * clean and invalidate one level cache.
+ *
+ * x0: cache level
+ * x1~x9: clobbered
+ */
+ENTRY(__asm_flush_dcache_level)
+