[Qemu-devel] [PATCH RFC v1 01/29] target-arc: initial commit

[Michael Rolnik]

Signed-off-by: Michael Rolnik 
---
 .gitignore  |   2 +
 MAINTAINERS |   5 +
 arch_init.c |   2 +
 configure   |   5 +
 default-configs/arc-softmmu.mak |   0
 include/sysemu/arch_init.h  |   1 +
 target-arc/Makefile.objs|  26 +++
 target-arc/cpu-qom.h|  84 
 target-arc/cpu.c| 269 +
 target-arc/cpu.h| 174 +
 target-arc/decode.c |   7 +
 target-arc/gdbstub.c| 138 +
 target-arc/helper.c |  74 +++
 target-arc/helper.h |  21 ++
 target-arc/machine.c|  35 
 target-arc/machine.h|  21 ++
 target-arc/translate.c  | 424 
 target-arc/translate.h  | 223 +
 18 files changed, 1511 insertions(+)
 create mode 100644 default-configs/arc-softmmu.mak
 create mode 100644 target-arc/Makefile.objs
 create mode 100644 target-arc/cpu-qom.h
 create mode 100644 target-arc/cpu.c
 create mode 100644 target-arc/cpu.h
 create mode 100644 target-arc/decode.c
 create mode 100644 target-arc/gdbstub.c
 create mode 100644 target-arc/helper.c
 create mode 100644 target-arc/helper.h
 create mode 100644 target-arc/machine.c
 create mode 100644 target-arc/machine.h
 create mode 100644 target-arc/translate.c
 create mode 100644 target-arc/translate.h

diff --git a/.gitignore b/.gitignore
index 88ec249..37a71af 100644
--- a/.gitignore
+++ b/.gitignore
@@ -58,6 +58,8 @@
 /fsdev/virtfs-proxy-helper
 *.[1-9]
 *.a
+*.swp
+*.swo
 *.aux
 *.cp
 *.dvi
diff --git a/MAINTAINERS b/MAINTAINERS
index b6fb84e..0500cf5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -99,6 +99,11 @@ F: hw/alpha/
 F: tests/tcg/alpha/
 F: disas/alpha.c
 
+ARC
+M: Michael Rolnik 
+S: Maintained
+F: target-arc/
+
 ARM
 M: Peter Maydell 
 L: qemu-...@nongnu.org
diff --git a/arch_init.c b/arch_init.c
index fa05973..04b51f5 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -80,6 +80,8 @@ int graphic_depth = 32;
 #define QEMU_ARCH QEMU_ARCH_UNICORE32
 #elif defined(TARGET_TRICORE)
 #define QEMU_ARCH QEMU_ARCH_TRICORE
+#elif defined(TARGET_ARC)
+#define QEMU_ARCH QEMU_ARCH_ARC
 #endif
 
 const uint32_t arch_type = QEMU_ARCH;
diff --git a/configure b/configure
index 5a9bda1..8aee641 100755
--- a/configure
+++ b/configure
@@ -5672,6 +5672,8 @@ case "$target_name" in
   ;;
   alpha)
   ;;
+  arc)
+  ;;
   arm|armeb)
 TARGET_ARCH=arm
 bflt="yes"
@@ -5874,6 +5876,9 @@ for i in $ARCH $TARGET_BASE_ARCH ; do
   disas_config "ARM_A64"
 fi
   ;;
+  arc)
+disas_config "ARC"
+  ;;
   arm)
 disas_config "ARM"
 if test -n "${cxx}"; then
diff --git a/default-configs/arc-softmmu.mak b/default-configs/arc-softmmu.mak
new file mode 100644
index 000..e69de29
diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
index 1c9dad1..35148a6 100644
--- a/include/sysemu/arch_init.h
+++ b/include/sysemu/arch_init.h
@@ -23,6 +23,7 @@ enum {
 QEMU_ARCH_UNICORE32 = (1 << 14),
 QEMU_ARCH_MOXIE = (1 << 15),
 QEMU_ARCH_TRICORE = (1 << 16),
+QEMU_ARCH_ARC = (1 << 16),
 };
 
 extern const uint32_t arch_type;
diff --git a/target-arc/Makefile.objs b/target-arc/Makefile.objs
new file mode 100644
index 000..a3475dd
--- /dev/null
+++ b/target-arc/Makefile.objs
@@ -0,0 +1,26 @@
+#
+#  QEMU ARC CPU
+#
+#  Copyright (c) 2016 Michael Rolnik
+#
+#  This library is free software; you can redistribute it and/or
+#  modify it under the terms of the GNU Lesser General Public
+#  License as published by the Free Software Foundation; either
+#  version 2.1 of the License, or (at your option) any later version.
+#
+#  This library 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
+#  Lesser General Public License for more details.
+#
+#  You should have received a copy of the GNU Lesser General Public
+#  License along with this library; if not, see
+#  
+#
+
+obj-y   += translate.o
+obj-y   += helper.o
+obj-y   += cpu.o
+obj-y   += gdbstub.o
+obj-y   += decode.o
+obj-$(CONFIG_SOFTMMU) += machine.o
diff --git a/target-arc/cpu-qom.h b/target-arc/cpu-qom.h
new file mode 100644
index 000..b9cb1b2
--- /dev/null
+++ b/target-arc/cpu-qom.h
@@ -0,0 +1,84 @@
+/*
+ * QEMU ARC CPU
+ *
+ * Copyright (c) 2016 Michael Rolnik
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library 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 PARTI

Re: [Qemu-devel] [PATCH RFC v1 01/29] target-arc: initial commit

[Richard Henderson]

On 09/08/2016 03:31 PM, Michael Rolnik wrote:

+#define CPU_IMM(env)((env)->r[62])
+#define CPU_PCL(env)((env)->r[63])


You don't need to represent these as actual registers.  These are better 
considered placeholder regnums to be filled in with actual values during 
translation.



+struct CPUARCState {
+uint32_tr[64];
+
+struct {
+uint32_tLf;
+uint32_tZf; /*  zero*/
+uint32_tNf; /*  negative*/
+uint32_tCf; /*  carry   */
+uint32_tVf; /*  overflow*/
+uint32_tUf;
+
+uint32_tDEf;
+uint32_tAEf;
+uint32_tA2f;/*  interrupt 1 is active   */
+uint32_tA1f;/*  interrupt 2 is active   */
+uint32_tE2f;/*  interrupt 1 mask*/
+uint32_tE1f;/*  interrupt 2 mask*/
+uint32_tHf; /*  halt*/
+} stat, stat_l1, stat_l2, stat_er;


There is no reason to represent each bit as a whole word, and even less to have 
four copies.


Only the current NZCV bits are relevant for expansion to a word, and even then 
you should consider not canonicalizing N, Z and V to a pure boolean value.



+
+struct {
+uint32_tS2;
+uint32_tS1;
+uint32_tCS;
+} macmod;


Does CS really need to be represented at all?  It appears to me to be a field 
that you write to that clears S1 and S2.



+switch (n) {
+case 0x00 ... 0x3f: {
+val = env->r[n];
+break;
+}


Please use the proper format for all switch statements.

switch (n) {
case 0x00 ... 0x3f:
val = env->r[n];
break;


+
+TCGv_env cpu_env;
+
+TCGv cpu_gp;/*  Global Pointer  */
+TCGv cpu_fp;/*  Frame Pointer   */
+TCGv cpu_sp;/*  Stack Pointer   */
+TCGv cpu_ilink1;/*  Level 1 interrupt link register */
+TCGv cpu_ilink2;/*  Level 2 interrupt link register */
+TCGv cpu_blink; /*  Branch link register*/
+TCGv cpu_mlo;   /*  Multiply low 32 bits, read only */
+TCGv cpu_mmi;   /*  Multiply middle 32 bits, read only  */
+TCGv cpu_mhi;   /*  Multiply high 32 bits, read only*/


Why are these separate variables?  They overlap cpu_r[N].  If you want them as 
names for clarity in translation, #define seems good enough.



+int arc_gen_INVALID(DisasCtxt *ctx)
+{
+printf("invalid inst @:%08x\n", ctx->cpc);


No printf.  It's not useful, even temporarily.


+ctx.zero = tcg_const_local_i32(0);
+ctx.one = tcg_const_local_i32(1);
+ctx.msb32 = tcg_const_local_i32(0x8000);
+ctx.msb16 = tcg_const_local_i32(0x8000);
+ctx.smax16 = tcg_const_local_i32(0x7fff);
+ctx.smax32 = tcg_const_local_i32(0x7fff);
+ctx.smax5 = tcg_const_local_i32(0x001f);
+ctx.smin5 = tcg_const_local_i32(0xffe1);


Why are you creating all of these?  Creating local temps containing immediates 
is a horrible waste.



+if (ctx.npc == env->lpe) {


You can't look at the contents of ENV during translation.

You'll need to implement this feature similar to how it's done for xtensa.  See 
helper_wsr_lbeg, helper_wsr_lend, and gen_check_loop_end.



r~

Re: [Qemu-devel] [PATCH RFC v1 01/29] target-arc: initial commit

[Max Filippov]

Hello,

On Tue, Sep 20, 2016 at 4:31 PM, Richard Henderson  wrote:
>> +if (ctx.npc == env->lpe) {
> You can't look at the contents of ENV during translation.
>
> You'll need to implement this feature similar to how it's done for xtensa.
> See helper_wsr_lbeg, helper_wsr_lend, and gen_check_loop_end.

I think it's fairly expensive solution, as it allows to have only one TB with
zero overhead loop at a time. I'm looking at ways to optimize it,
perhaps recording lbeg in the TB's cs_base.

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH RFC v1 01/29] target-arc: initial commit

[Richard Henderson]

On 09/25/2016 06:22 PM, Max Filippov wrote:
> Hello,
> 
> On Tue, Sep 20, 2016 at 4:31 PM, Richard Henderson  wrote:
>>> +if (ctx.npc == env->lpe) {
>> You can't look at the contents of ENV during translation.
>>
>> You'll need to implement this feature similar to how it's done for xtensa.
>> See helper_wsr_lbeg, helper_wsr_lend, and gen_check_loop_end.
> 
> I think it's fairly expensive solution, as it allows to have only one TB with
> zero overhead loop at a time. I'm looking at ways to optimize it,
> perhaps recording lbeg in the TB's cs_base.
> 

It may be expensive, but at least it works.  Unlike the mechanism quoted above,
which doesn't work at all.


r~