qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Roman Kapl <rka@sysgo.com>
Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] ppc: fix crash during branch stepping
Date: Wed, 13 Feb 2019 12:35:12 +1100	[thread overview]
Message-ID: <20190213013511.GW1884@umbus.fritz.box> (raw)
In-Reply-To: <20190212121255.2279-1-rka@sysgo.com>

[-- Attachment #1: Type: text/plain, Size: 4080 bytes --]

On Tue, Feb 12, 2019 at 01:12:55PM +0100, Roman Kapl wrote:
> The PPC BRANCH exception could bubble up, but this is an QEMU internal exception
> and QEMU then crased. Instead it should trigger TRACE exception, according to
> PPC 2.07 book. It could happen only when using branch stepping, which is not
> commonly used.
> 
> Change gen_prep_dbgex do do trigger TRACE. The excp, argument is now removed,
> since the type of exception can be inferred from the singlestep_enabled flags.
> removed the guards around gen_exception, since they are unnecessary.
> 
> Fixes: 0e3bf48909 ("ppc: add DBCR based debugging").
> Signed-off-by: Roman Kapl <rka@sysgo.com>

Applied to ppc-for-4.0, thanks.

> ---
>  target/ppc/translate.c | 37 +++++++++++++++----------------------
>  1 file changed, 15 insertions(+), 22 deletions(-)
> 
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index e169c43643..c22d1a69c7 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -287,26 +287,22 @@ static void gen_exception_nip(DisasContext *ctx, uint32_t excp,
>      ctx->exception = (excp);
>  }
>  
> -/* Translates the EXCP_TRACE/BRANCH exceptions used on most PowerPCs to
> - * EXCP_DEBUG, if we are running on cores using the debug enable bit (e.g.
> - * BookE).
> +/*
> + * Tells the caller what is the appropriate exception to generate and prepares
> + * SPR registers for this exception.
> + *
> + * The exception can be either POWERPC_EXCP_TRACE (on most PowerPCs) or
> + * POWERPC_EXCP_DEBUG (on BookE).
>   */
> -static uint32_t gen_prep_dbgex(DisasContext *ctx, uint32_t excp)
> +static uint32_t gen_prep_dbgex(DisasContext *ctx)
>  {
> -    if ((ctx->singlestep_enabled & CPU_SINGLE_STEP)
> -        && (excp == POWERPC_EXCP_BRANCH)) {
> -        /* Trace excpt. has priority */
> -        excp = POWERPC_EXCP_TRACE;
> -    }
>      if (ctx->flags & POWERPC_FLAG_DE) {
>          target_ulong dbsr = 0;
> -        switch (excp) {
> -        case POWERPC_EXCP_TRACE:
> +        if (ctx->singlestep_enabled & CPU_SINGLE_STEP) {
>              dbsr = DBCR0_ICMP;
> -            break;
> -        case POWERPC_EXCP_BRANCH:
> +        } else {
> +            /* Must have been branch */
>              dbsr = DBCR0_BRT;
> -            break;
>          }
>          TCGv t0 = tcg_temp_new();
>          gen_load_spr(t0, SPR_BOOKE_DBSR);
> @@ -315,7 +311,7 @@ static uint32_t gen_prep_dbgex(DisasContext *ctx, uint32_t excp)
>          tcg_temp_free(t0);
>          return POWERPC_EXCP_DEBUG;
>      } else {
> -        return excp;
> +        return POWERPC_EXCP_TRACE;
>      }
>  }
>  
> @@ -3652,10 +3648,8 @@ static void gen_lookup_and_goto_ptr(DisasContext *ctx)
>          if (sse & GDBSTUB_SINGLE_STEP) {
>              gen_debug_exception(ctx);
>          } else if (sse & (CPU_SINGLE_STEP | CPU_BRANCH_STEP)) {
> -            uint32_t excp = gen_prep_dbgex(ctx, POWERPC_EXCP_BRANCH);
> -            if (excp != POWERPC_EXCP_NONE) {
> -                gen_exception(ctx, excp);
> -            }
> +            uint32_t excp = gen_prep_dbgex(ctx);
> +            gen_exception(ctx, excp);
>          }
>          tcg_gen_exit_tb(NULL, 0);
>      } else {
> @@ -7785,9 +7779,8 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
>                   ctx->exception != POWERPC_SYSCALL &&
>                   ctx->exception != POWERPC_EXCP_TRAP &&
>                   ctx->exception != POWERPC_EXCP_BRANCH)) {
> -        uint32_t excp = gen_prep_dbgex(ctx, POWERPC_EXCP_TRACE);
> -        if (excp != POWERPC_EXCP_NONE)
> -            gen_exception_nip(ctx, excp, ctx->base.pc_next);
> +        uint32_t excp = gen_prep_dbgex(ctx);
> +        gen_exception_nip(ctx, excp, ctx->base.pc_next);
>      }
>  
>      if (tcg_check_temp_count()) {

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

      reply	other threads:[~2019-02-13  4:14 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-12 12:12 [Qemu-devel] [PATCH] ppc: fix crash during branch stepping Roman Kapl
2019-02-13  1:35 ` David Gibson [this message]

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=20190213013511.GW1884@umbus.fritz.box \
    --to=david@gibson.dropbear.id.au \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    --cc=rka@sysgo.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).