qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Fabiano Rosas <farosas@linux.ibm.com>
To: "Bruno Larsen (billionai)" <bruno.larsen@eldorado.org.br>,
	qemu-devel@nongnu.org
Cc: luis.pires@eldorado.org.br, lucas.araujo@eldorado.org.br,
	fernando.valle@eldorado.org.br,
	"Bruno Larsen \(billionai\)" <bruno.larsen@eldorado.org.br>,
	matheus.ferst@eldorado.org.br, david@gibson.dropbear.id.au
Subject: Re: [PATCH] target/ppc: code motion from translate_init.c.inc to gdbstub.c
Date: Mon, 12 Apr 2021 18:00:39 -0300	[thread overview]
Message-ID: <874kgb5ibs.fsf@linux.ibm.com> (raw)
In-Reply-To: <20210412190442.114467-1-bruno.larsen@eldorado.org.br>

"Bruno Larsen (billionai)" <bruno.larsen@eldorado.org.br> writes:

Please send ppc patches to both qemu-devel and qemu-ppc.

> As suggested by Fabiano Rosas,

In these situations you can just add along with your signed-off-by:

Suggested-by: Fabiano Rosas <farosas@linux.ibm.com>

> all the code related to gdb has been moved
> from translate_init.c.inc file to the gdbstub.c file, where it makes more
> sense
>
> Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br>
> ---
>  target/ppc/cpu.h                |  11 ++
>  target/ppc/gdbstub.c            | 261 ++++++++++++++++++++++++++++++++
>  target/ppc/translate_init.c.inc | 224 ---------------------------
>  3 files changed, 272 insertions(+), 224 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index e73416da68..795b121e04 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -2612,4 +2612,15 @@ static inline ppc_avr_t *cpu_avr_ptr(CPUPPCState *env, int i)
>  void dump_mmu(CPUPPCState *env);
>  
>  void ppc_maybe_bswap_register(CPUPPCState *env, uint8_t *mem_buf, int len);
> +
> +/*
> + * functions used by cpu_ppc_realize, but that dont necessarily make sense
> + * to be added to cpu.c, because they seem very related to TCG or gdb
> + */

This comment has served its purpose for the RFC I think. You can drop it.

> +
> +/* gdbstub.c */
> +void ppc_cpu_gdb_init(CPUState *cs, PowerPCCPUClass *ppc);

I don't think "cpu" adds much here. ppc_gdb_init gets the meaning across
just fine.

> +gchar *ppc_gdb_arch_name(CPUState *cs);
> +
> +
>  #endif /* PPC_CPU_H */
> diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c
> index c28319fb97..0c016b8483 100644
> --- a/target/ppc/gdbstub.c
> +++ b/target/ppc/gdbstub.c
> @@ -20,6 +20,10 @@
>  #include "qemu/osdep.h"
>  #include "cpu.h"
>  #include "exec/gdbstub.h"
> +#ifdef CONFIG_TCG
> +#include "exec/helper-proto.h"
> +#endif

We still need to figure out where to move the vscr helpers so that both
TCG and !TCG code can see them. But we cannot build without TCG
currently anyway so I guess it's ok to leave the ifdef.

> +#include "kvm_ppc.h"

This one is not being used.

>  
>  static int ppc_gdb_register_len_apple(int n)
>  {
> @@ -387,3 +391,260 @@ const char *ppc_gdb_get_dynamic_xml(CPUState *cs, const char *xml_name)
>      return NULL;
>  }
>  #endif

<snip>

> +gchar *ppc_gdb_arch_name(CPUState *cs)
> +{
> +#if defined(TARGET_PPC64)
> +    return g_strdup("powerpc:common64");
> +#else
> +    return g_strdup("powerpc:common");
> +#endif
> +}

Where is the removal of this from translate_init.inc.c? You must have
left it unstaged in your tree.

> +
> +void ppc_cpu_gdb_init(CPUState *cs, PowerPCCPUClass *pcc)
> +{
> +
> +    if (pcc->insns_flags & PPC_FLOAT) {
> +        gdb_register_coprocessor(cs, gdb_get_float_reg, gdb_set_float_reg,
> +                                 33, "power-fpu.xml", 0);
> +    }
> +    if (pcc->insns_flags & PPC_ALTIVEC) {
> +        gdb_register_coprocessor(cs, gdb_get_avr_reg, gdb_set_avr_reg,
> +                                 34, "power-altivec.xml", 0);
> +    }
> +    if (pcc->insns_flags & PPC_SPE) {
> +        gdb_register_coprocessor(cs, gdb_get_spe_reg, gdb_set_spe_reg,
> +                                 34, "power-spe.xml", 0);
> +    }
> +    if (pcc->insns_flags2 & PPC2_VSX) {
> +        gdb_register_coprocessor(cs, gdb_get_vsx_reg, gdb_set_vsx_reg,
> +                                 32, "power-vsx.xml", 0);
> +    }
> +#ifndef CONFIG_USER_ONLY
> +    gdb_register_coprocessor(cs, gdb_get_spr_reg, gdb_set_spr_reg,
> +                             pcc->gdb_num_sprs, "power-spr.xml", 0);
> +#endif
> +}

Same here.


  reply	other threads:[~2021-04-12 21:02 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-12 19:04 Bruno Larsen (billionai)
2021-04-12 21:00 ` Fabiano Rosas [this message]
2021-04-13 12:48   ` Bruno Piazera Larsen
2021-04-13 13:31     ` Fabiano Rosas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=874kgb5ibs.fsf@linux.ibm.com \
    --to=farosas@linux.ibm.com \
    --cc=bruno.larsen@eldorado.org.br \
    --cc=david@gibson.dropbear.id.au \
    --cc=fernando.valle@eldorado.org.br \
    --cc=lucas.araujo@eldorado.org.br \
    --cc=luis.pires@eldorado.org.br \
    --cc=matheus.ferst@eldorado.org.br \
    --cc=qemu-devel@nongnu.org \
    --subject='Re: [PATCH] target/ppc: code motion from translate_init.c.inc to gdbstub.c' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).