Patchwork [03/11] S/390 fake TCG implementation

login
register
mail settings
Submitter Alexander Graf
Date Nov. 26, 2009, 1:23 p.m.
Message ID <1259241800-2810-4-git-send-email-agraf@suse.de>
Download mbox | patch
Permalink /patch/39517/
State New
Headers show

Comments

Alexander Graf - Nov. 26, 2009, 1:23 p.m.
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.

Signed-off-by: Alexander Graf <agraf@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
Aurelien Jarno - Nov. 30, 2009, 6:18 p.m.
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 <agraf@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.

>  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 <uli@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.

> +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 <uli@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.

> +/* 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
> 
> 
> 
>
Alexander Graf - Nov. 30, 2009, 10:27 p.m.
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 <agraf@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?

>> 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 <uli@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.

> 
>> +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 <uli@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?

>> +/* 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.

Oh, ok :)

Alex
Aurelien Jarno - Dec. 2, 2009, 8:16 a.m.
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 <agraf@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.

> >> 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 <uli@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.

> > 
> >> +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 <uli@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.

> >> +/* 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.
> 
> Oh, ok :)
> 
> Alex
Alexander Graf - Dec. 2, 2009, 8:29 a.m.
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 <agraf@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 <uli@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 <uli@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.
Aurelien Jarno - Dec. 2, 2009, 8:41 a.m.
On Wed, Dec 02, 2009 at 09:29:59AM +0100, Alexander Graf wrote:
> 
> 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 <agraf@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.

Then it's clearly not part of this patch, it should probably be part of
patch 1 or 6 instead.

> > 
> >>>> 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 <uli@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 <uli@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.
Alexander Graf - Dec. 2, 2009, 8:44 a.m.
On 02.12.2009, at 09:41, Aurelien Jarno wrote:

> On Wed, Dec 02, 2009 at 09:29:59AM +0100, Alexander Graf wrote:
>> 
>> 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 <agraf@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.
> 
> Then it's clearly not part of this patch, it should probably be part of
> patch 1 or 6 instead.

Yes, moved it already.

Alex

Patch

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;
+}
+
 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 <uli@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)
+{
+}
+
+void tcg_target_qemu_prologue(TCGContext *s)
+{
+}
+
+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 <uli@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,
+};
+
+/* 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);
+#else
+#error not implemented
+#endif
+}