On 02.12.2009, at 09:16, Aurelien Jarno wrote: > On Mon, Nov 30, 2009 at 11:27:53PM +0100, Alexander Graf wrote: >> >> On 30.11.2009, at 19:18, Aurelien Jarno wrote: >> >>> On Thu, Nov 26, 2009 at 02:23:12PM +0100, Alexander Graf wrote: >>>> Qemu won't let us run a KVM target without having host TCG support. Well, >>>> for >>>> now we don't have any so let's implement a fake target that only stubs out >>>> everything. >>>> >>>> I tried to keep the patch as close to Uli's source as possible, so whenever >>>> he feels like it he can easily diff his version against this one. >>> >>> Please find the comments below. >>> >>>> Signed-off-by: Alexander Graf <ag...@suse.de> >>>> --- >>>> dyngen-exec.h | 2 +- >>>> target-s390x/helper.c | 5 ++ >>>> tcg/s390/tcg-target.c | 103 >>>> +++++++++++++++++++++++++++++++++++++++++++++++++ >>>> tcg/s390/tcg-target.h | 48 +++++++++++++++++++++++ >>>> 4 files changed, 157 insertions(+), 1 deletions(-) >>>> create mode 100644 tcg/s390/tcg-target.c >>>> create mode 100644 tcg/s390/tcg-target.h >>>> >>>> diff --git a/dyngen-exec.h b/dyngen-exec.h >>>> index 86e61c3..0353f36 100644 >>>> --- a/dyngen-exec.h >>>> +++ b/dyngen-exec.h >>>> @@ -117,7 +117,7 @@ extern int printf(const char *, ...); >>>> >>>> /* The return address may point to the start of the next instruction. >>>> Subtracting one gets us the call instruction itself. */ >>>> -#if defined(__s390__) >>>> +#if defined(__s390__) && !defined(__s390x__) >>>> # define GETPC() ((void*)(((unsigned long)__builtin_return_address(0) & >>>> 0x7fffffffUL) - 1)) >>>> #elif defined(__arm__) >>>> /* Thumb return addresses have the low bit set, so we need to subtract two. >>>> diff --git a/target-s390x/helper.c b/target-s390x/helper.c >>>> index 4e23b4a..0e222e3 100644 >>>> --- a/target-s390x/helper.c >>>> +++ b/target-s390x/helper.c >>>> @@ -44,6 +44,11 @@ CPUS390XState *cpu_s390x_init(const char *cpu_model) >>>> return env; >>>> } >>>> >>>> +target_phys_addr_t cpu_get_phys_page_debug(CPUState *env, target_ulong >>>> addr) >>>> +{ >>>> + return addr; >>>> +} >>>> + >>> >>> Why does it appear in this patch? It has nothing to do with TCG support. >> >> I don't remember. What is this used for anyways? > > If it is not used, maybe it's better to remove it.
It's called from exec.c. > >>>> void cpu_reset(CPUS390XState *env) >>>> { >>>> if (qemu_loglevel_mask(CPU_LOG_RESET)) { >>>> diff --git a/tcg/s390/tcg-target.c b/tcg/s390/tcg-target.c >>>> new file mode 100644 >>>> index 0000000..53bbf69 >>>> --- /dev/null >>>> +++ b/tcg/s390/tcg-target.c >>>> @@ -0,0 +1,103 @@ >>>> +/* >>>> + * Tiny Code Generator for QEMU >>>> + * >>>> + * Copyright (c) 2009 Ulrich Hecht <u...@suse.de> >>>> + * >>>> + * Permission is hereby granted, free of charge, to any person obtaining >>>> a copy >>>> + * of this software and associated documentation files (the "Software"), >>>> to deal >>>> + * in the Software without restriction, including without limitation the >>>> rights >>>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or >>>> sell >>>> + * copies of the Software, and to permit persons to whom the Software is >>>> + * furnished to do so, subject to the following conditions: >>>> + * >>>> + * The above copyright notice and this permission notice shall be >>>> included in >>>> + * all copies or substantial portions of the Software. >>>> + * >>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, >>>> EXPRESS OR >>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >>>> MERCHANTABILITY, >>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL >>>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR >>>> OTHER >>>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, >>>> ARISING FROM, >>>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS >>>> IN >>>> + * THE SOFTWARE. >>>> + */ >>>> + >>>> +static const int tcg_target_reg_alloc_order[] = { >>>> +}; >>>> + >>>> +static const int tcg_target_call_iarg_regs[] = { >>>> +}; >>>> + >>>> +static const int tcg_target_call_oarg_regs[] = { >>>> +}; >>>> + >>>> +static void patch_reloc(uint8_t *code_ptr, int type, >>>> + tcg_target_long value, tcg_target_long addend) >>>> +{ >>>> + tcg_abort(); >>>> +} >>>> + >>>> +static inline int tcg_target_get_call_iarg_regs_count(int flags) >>>> +{ >>>> + tcg_abort(); >>>> + return 0; >>>> +} >>>> + >>>> +/* parse target specific constraints */ >>>> +static int target_parse_constraint(TCGArgConstraint *ct, const char >>>> **pct_str) >>>> +{ >>>> + tcg_abort(); >>>> + return 0; >>>> +} >>>> + >>>> +/* Test if a constant matches the constraint. */ >>>> +static inline int tcg_target_const_match(tcg_target_long val, >>>> + const TCGArgConstraint *arg_ct) >>>> +{ >>>> + tcg_abort(); >>>> + return 0; >>>> +} >>>> + >>>> +/* load a register with an immediate value */ >>>> +static inline void tcg_out_movi(TCGContext *s, TCGType type, >>>> + int ret, tcg_target_long arg) >>>> +{ >>>> + tcg_abort(); >>>> +} >>>> + >>>> +/* load data without address translation or endianness conversion */ >>>> +static inline void tcg_out_ld(TCGContext *s, TCGType type, int arg, >>>> + int arg1, tcg_target_long arg2) >>>> +{ >>>> + tcg_abort(); >>>> +} >>>> + >>>> +static inline void tcg_out_st(TCGContext *s, TCGType type, int arg, >>>> + int arg1, tcg_target_long arg2) >>>> +{ >>>> + tcg_abort(); >>>> +} >>>> + >>>> +static inline void tcg_out_op(TCGContext *s, int opc, >>>> + const TCGArg *args, const int *const_args) >>>> +{ >>>> + tcg_abort(); >>>> +} >>>> + >>>> +void tcg_target_init(TCGContext *s) >>>> +{ >>>> +} >>> >>> This is probably here that tcg_abort() is actually the most important. >>> >>>> + >>>> +void tcg_target_qemu_prologue(TCGContext *s) >>>> +{ >>>> +} >>>> + >>> >>> Also adding it here doesn't cost a lot. >> >> Both places get called with KVM as well. If I call tcg_abort() here KVM >> doesn't start. > > Then can you add a comment explaining that code is missing. > > I am very reluctant in introducing missing/wrong code in qemu. > Experience has shown that it stays for a very long time, until someone > spend hours or days trying to debug an issue. Alright. > >>> >>>> +static inline void tcg_out_mov(TCGContext *s, int ret, int arg) >>>> +{ >>>> + tcg_abort(); >>>> +} >>>> + >>>> +static inline void tcg_out_addi(TCGContext *s, int reg, tcg_target_long >>>> val) >>>> +{ >>>> + tcg_abort(); >>>> +} >>>> diff --git a/tcg/s390/tcg-target.h b/tcg/s390/tcg-target.h >>>> new file mode 100644 >>>> index 0000000..2e66553 >>>> --- /dev/null >>>> +++ b/tcg/s390/tcg-target.h >>>> @@ -0,0 +1,48 @@ >>>> +/* >>>> + * Tiny Code Generator for QEMU >>>> + * >>>> + * Copyright (c) 2009 Ulrich Hecht <u...@suse.de> >>>> + * >>>> + * Permission is hereby granted, free of charge, to any person obtaining >>>> a copy >>>> + * of this software and associated documentation files (the "Software"), >>>> to deal >>>> + * in the Software without restriction, including without limitation the >>>> rights >>>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or >>>> sell >>>> + * copies of the Software, and to permit persons to whom the Software is >>>> + * furnished to do so, subject to the following conditions: >>>> + * >>>> + * The above copyright notice and this permission notice shall be >>>> included in >>>> + * all copies or substantial portions of the Software. >>>> + * >>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, >>>> EXPRESS OR >>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >>>> MERCHANTABILITY, >>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL >>>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR >>>> OTHER >>>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, >>>> ARISING FROM, >>>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS >>>> IN >>>> + * THE SOFTWARE. >>>> + */ >>>> +#define TCG_TARGET_S390 1 >>>> + >>>> +#define TCG_TARGET_REG_BITS 64 >>>> +#define TCG_TARGET_WORDS_BIGENDIAN >>>> + >>>> +#define TCG_TARGET_NB_REGS 0 >>>> + >>>> +enum { >>>> + TCG_AREG0 = 0, >>>> +}; >>> >>> As this is defined in Uli's patch, I think it might be a good idea to >>> define that correctly. Same for the list of registers. >> >> I wasn't sure how much of the real architecture I should actually keep. So >> you think registers belong in? > > Again what chokes me is that it introduces wrong code. Especially > TCG_AREG0 = 0. Introducing the registers will fix that. Ok.