Re: [Qemu-devel] [PATCH 03/11] S/390 fake TCG implementation

[Aurelien Jarno]

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 <address@hidden>
> ---
>  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.

>  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 <address@hidden>
> + *
> + * 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.

> +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 <address@hidden>
> + *
> + * 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.

> +/* used for function call generation */
> +#define TCG_REG_CALL_STACK           0
> +#define TCG_TARGET_STACK_ALIGN               8
> +#define TCG_TARGET_CALL_STACK_OFFSET 0
> +
> +static inline void flush_icache_range(unsigned long start, unsigned long 
> stop)
> +{
> +#if QEMU_GNUC_PREREQ(4, 1)
> +    void __clear_cache(char *beg, char *end);
> +    __clear_cache((char *) start, (char *) stop);

You should use __builtin___clear_cache instead to avoid having to
declare the function.

> +#else
> +#error not implemented
> +#endif
> +}
> -- 
> 1.6.0.2
> 
> 
> 
> 

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
address@hidden                 http://www.aurel32.net