From: "Pavel Dovgaluk" <Pavel.Dovgaluk@ispras.ru>
References: <20141107103123.6136.18545.stgit@PASHA-ISP>
	<20141107103230.6136.83421.stgit@PASHA-ISP>
	<5464BD52.1000803@redhat.com>
In-Reply-To: <5464BD52.1000803@redhat.com>
Date: Mon, 17 Nov 2014 12:35:08 +0300
Message-ID: <000901d00249$c6da7c60$548f7520$@Dovgaluk@ispras.ru>
MIME-Version: 1.0
Content-Type: text/plain;
	charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Content-Language: ru
Subject: Re: [Qemu-devel] [RFC PATCH v4 11/25] cpu-exec: allow temporary
	disabling icount
To: 'Paolo Bonzini' <pbonzini@redhat.com>, qemu-devel@nongnu.org
Cc: peter.maydell@linaro.org, peter.crosthwaite@xilinx.com, mark.burton@greensocs.com, real@ispras.ru, batuzovk@ispras.ru, maria.klimushenkova@ispras.ru, alex.bennee@linaro.org, afaerber@suse.de, fred.konrad@greensocs.com

> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of =
Paolo Bonzini
> On 07/11/2014 11:32, Pavel Dovgalyuk wrote:
> >          cpu_restore_state_from_tb(cpu, tb, retaddr);
> > +        /* tb could be temporary, generated by exec nocache */
> > +        tb_phys_invalidate(tb, -1);
>=20
> Would you need to do the same sequence as cpu_exec_nocache in this =
case?

Right.

>     cpu->current_tb =3D NULL;
>     tb_phys_invalidate(tb, -1);
>     tb_free(tb);
>=20
> In this case, perhaps something like this would be better:
>=20
> diff --git a/cpu-exec.c b/cpu-exec.c
> index 10c0f42..faf8041 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -209,7 +209,7 @@ static void cpu_exec_nocache(CPUArchState *env, =
int max_cycles,
>          max_cycles =3D CF_COUNT_MASK;
>=20
>      tb =3D tb_gen_code(cpu, orig_tb->pc, orig_tb->cs_base, =
orig_tb->flags,
> -                     max_cycles);
> +                     max_cycles | CF_NOCACHE);
>      cpu->current_tb =3D tb;
>      /* execute the generated code */
>      trace_exec_tb_nocache(tb, tb->pc);
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 9b67d16..41464f3 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -148,6 +148,7 @@ struct TranslationBlock {
>  #define CF_COUNT_MASK  0x7fff
>  #define CF_LAST_IO     0x8000 /* Last insn may be an IO access.  */
>  #define CF_LAST_IO     0x8000 /* Last insn may be an IO access.  */
> +#define CF_NOCACHE     0x40000 /* To be freed after execution */
>=20
>      void *tc_ptr;    /* pointer to the translated code */
>      /* next matching tb for physical address. */
> diff --git a/translate-all.c b/translate-all.c
> index 1a27df7..6ea4da4 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -264,6 +264,12 @@ bool cpu_restore_state(CPUState *cpu, uintptr_t =
retaddr)
>      tb =3D tb_find_pc(retaddr);
>      if (tb) {
>          cpu_restore_state_from_tb(cpu, tb, retaddr);
> +        if (tb->cflags & CF_NOCACHE) {
> +            /* one-shot translation, invalidate it immediately */
> +            cpu->current_tb =3D NULL;
> +            tb_phys_invalidate(tb, -1);
> +            tb_free(tb);
> +        }
>          return true;
>      }
>      return false;
>=20
>=20
> I pushed it to an icount branch on my github repository, together with
> other patches that introduce CF_USE_ICOUNT instead of touching the
> use_icount global variable.  Can you please take a look and, if it
> works, incorporate it in your patch series?

Ok, I'll check it.
I noticed that you missed this patch in your branch: =
http://lists.nongnu.org/archive/html/qemu-devel/2014-10/msg04034.html

Pavel Dovgalyuk