On 7 July 2014 19:13, Bastian Koppelmann <kbast...@mail.uni-paderborn.de> wrote:
> Add TriCore target stubs, QOM cpu and basic machine.
>
> Signed-off-by: Bastian Koppelmann <kbast...@mail.uni-paderborn.de>
> ---
> arch_init.c | 2 +
> configure | 13 ++
> cpu-exec.c | 11 +-
> cpus.c | 6 +
> default-configs/tricore-softmmu.mak | 3 +
> include/elf.h | 2 +
> include/sysemu/arch_init.h | 1 +
> target-tricore/Makefile.objs | 2 +
> target-tricore/cpu-qom.h | 71 +++++++
> target-tricore/cpu.c | 121 ++++++++++++
> target-tricore/cpu.h | 380
> ++++++++++++++++++++++++++++++++++++
> target-tricore/helper.c | 36 ++++
> target-tricore/helper.h | 0
> target-tricore/machine.c | 21 ++
> target-tricore/op_helper.c | 27 +++
> target-tricore/translate.c | 108 ++++++++++
> target-tricore/translate_init.c | 21 ++
> target-tricore/tricore-defs.h | 28 +++
> user-exec.c | 17 ++
> 19 files changed, 869 insertions(+), 1 deletion(-)
> create mode 100644 default-configs/tricore-softmmu.mak
> create mode 100644 target-tricore/Makefile.objs
> create mode 100644 target-tricore/cpu-qom.h
> create mode 100644 target-tricore/cpu.c
> create mode 100644 target-tricore/cpu.h
> create mode 100644 target-tricore/helper.c
> create mode 100644 target-tricore/helper.h
> create mode 100644 target-tricore/machine.c
> create mode 100644 target-tricore/op_helper.c
> create mode 100644 target-tricore/translate.c
> create mode 100644 target-tricore/translate_init.c
> create mode 100644 target-tricore/tricore-defs.h
I think you probably want the configure and default-configs changes
to be at the end of the patch series (or at least part way through it),
not at the start.
The general approach is:
* some patches which implement the basic absolute minimum
functionality, usefully split up
* then add the configure and default-configs changes that allow
the user to select the target and include it in the default target
list
> diff --git a/configure b/configure
> index 7dd43fd..a976862 100755
> --- a/configure
> +++ b/configure
> @@ -495,6 +495,8 @@ elif check_define __mips__ ; then
> cpu="mips"
> elif check_define __ia64__ ; then
> cpu="ia64"
> +elif check_define __tricore__ ; then
> + cpu="tricore"
> elif check_define __s390__ ; then
> if check_define __s390x__ ; then
> cpu="s390x"
This is for supporting compilation of QEMU on a tricore
*host*. That is a bit pointless since you don't provide a
TCG target backend. In any case if you want to add support
for that I suggest restricting it to its own separate patchset.
> @@ -533,6 +535,9 @@ case "$cpu" in
> mips*)
> cpu="mips"
> ;;
> + tricore*)
> + cpu="tricore"
> + ;;
Also host CPU related. (I suggest looking at where the
configure script handles the "moxie" target type, since that's
an example of a CPU type we handle only as a guest CPU
and not as a host CPU, so it's a good indication of where you
need to add tricore cases.)
> sparc|sun4[cdmuv])
> cpu="sparc"
> ;;
> @@ -4958,6 +4963,10 @@ case "$target_name" in
> TARGET_BASE_ARCH=mips
> echo "TARGET_ABI_MIPSN64=y" >> $config_target_mak
> ;;
> + tricore)
> + TARGET_ARCH=tricore
You don't need this line, because the default value for
TARGET_ARCH is the target name; only those cases in this
case statement where the CPU target name might be different
need to set TARGET_ARCH specifically.
> + target_phys_bits=32
> + ;;
> moxie)
> ;;
> or32)
> @@ -5155,6 +5164,10 @@ for i in $ARCH $TARGET_BASE_ARCH ; do
> echo "CONFIG_MIPS_DIS=y" >> $config_target_mak
> echo "CONFIG_MIPS_DIS=y" >> config-all-disas.mak
> ;;
> + tricore*)
> + echo "CONFIG_TRICORE_DIS=y" >> $config_target_mak
> + echo "CONFIG_TRICORE_DIS=y" >> config-all-disas.mak
> + ;;
Diffstat says you haven't got a disassembler in this patch,
so what is this for? Don't add it until/unless you add the
disassembler code.
> --- /dev/null
> +++ b/target-tricore/machine.c
> @@ -0,0 +1,21 @@
> +/*
> + * Copyright (c) 2012-2014 Bastian Koppelmann C-Lab/University Paderborn
> + *
> + * 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 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
> <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "hw/hw.h"
> +#include "hw/boards.h"
> +
> +#include "cpu.h"
This file has no content; don't add it until it's got some useful code in it...
> diff --git a/user-exec.c b/user-exec.c
> index 1ff8673..beba4d5 100644
> --- a/user-exec.c
> +++ b/user-exec.c
> @@ -614,6 +614,23 @@ int cpu_signal_handler(int host_signum, void *pinfo,
> is_write, &uc->uc_sigmask, puc);
> }
>
> +#elif defined(__tricore__)
> +
> +int cpu_signal_handler(int host_signum, void *pinfo,
> + void *puc)
> +{
> + printf("cpu_signal_handler\n");
> + /* siginfo_t *info = pinfo;
> + struct ucontext *uc = puc;
> + greg_t pc = uc->uc_mcontext.pc;
> + int is_write;
> + is_write = 0;
> + return handle_cpu_signal(pc, (unsigned long)info->si_addr,
> + is_write, &uc->uc_sigmask, puc);*/
> + return 0;
> +}
> +
> +
This is also host-CPU support; don't put it in this patchset.
(Also, don't include commented out code like that...)
thanks
-- PMM
