Re: [Qemu-devel] [PATCH for-4.1 v2 01/36] tcg: Fold CPUTLBWindow into CPUTLBDesc

Re: [Qemu-devel] [PATCH for-4.1 v2 01/36] tcg: Fold CPUTLBWindow into CPUTLBDesc

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  include/exec/cpu-defs.h | 17 ++++-------------
>  accel/tcg/cputlb.c      | 24 ++++++++++++------------
>  2 files changed, 16 insertions(+), 25 deletions(-)
>
> diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h
> index 8f2a848bf5..52d150aaf1 100644
> --- a/include/exec/cpu-defs.h
> +++ b/include/exec/cpu-defs.h
> @@ -127,18 +127,6 @@ typedef struct CPUIOTLBEntry {
>      MemTxAttrs attrs;
>  } CPUIOTLBEntry;
>
> -/**
> - * struct CPUTLBWindow
> - * @begin_ns: host time (in ns) at the beginning of the time window
> - * @max_entries: maximum number of entries observed in the window
> - *
> - * See also: tlb_mmu_resize_locked()
> - */
> -typedef struct CPUTLBWindow {
> -    int64_t begin_ns;
> -    size_t max_entries;
> -} CPUTLBWindow;
> -
>  typedef struct CPUTLBDesc {
>      /*
>       * Describe a region covering all of the large pages allocated
> @@ -148,9 +136,12 @@ typedef struct CPUTLBDesc {
>       */
>      target_ulong large_page_addr;
>      target_ulong large_page_mask;
> +    /* host time (in ns) at the beginning of the time window */
> +    int64_t window_begin_ns;
> +    /* maximum number of entries observed in the window */
> +    size_t window_max_entries;
>      /* The next index to use in the tlb victim table.  */
>      size_t vindex;
> -    CPUTLBWindow window;
>      size_t n_used_entries;
>  } CPUTLBDesc;
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 88cc8389e9..23586f9974 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -79,11 +79,11 @@ static inline size_t sizeof_tlb(CPUArchState *env, uintptr_t mmu_idx)
>      return env->tlb_mask[mmu_idx] + (1 << CPU_TLB_ENTRY_BITS);
>  }
>
> -static void tlb_window_reset(CPUTLBWindow *window, int64_t ns,
> +static void tlb_window_reset(CPUTLBDesc *desc, int64_t ns,
>                               size_t max_entries)
>  {
> -    window->begin_ns = ns;
> -    window->max_entries = max_entries;
> +    desc->window_begin_ns = ns;
> +    desc->window_max_entries = max_entries;
>  }
>
>  static void tlb_dyn_init(CPUArchState *env)
> @@ -94,7 +94,7 @@ static void tlb_dyn_init(CPUArchState *env)
>          CPUTLBDesc *desc = &env->tlb_d[i];
>          size_t n_entries = 1 << CPU_TLB_DYN_DEFAULT_BITS;
>
> -        tlb_window_reset(&desc->window, get_clock_realtime(), 0);
> +        tlb_window_reset(desc, get_clock_realtime(), 0);
>          desc->n_used_entries = 0;
>          env->tlb_mask[i] = (n_entries - 1) << CPU_TLB_ENTRY_BITS;
>          env->tlb_table[i] = g_new(CPUTLBEntry, n_entries);
> @@ -151,18 +151,18 @@ static void tlb_mmu_resize_locked(CPUArchState *env, int mmu_idx)
>      int64_t now = get_clock_realtime();
>      int64_t window_len_ms = 100;
>      int64_t window_len_ns = window_len_ms * 1000 * 1000;
> -    bool window_expired = now > desc->window.begin_ns + window_len_ns;
> +    bool window_expired = now > desc->window_begin_ns + window_len_ns;
>
> -    if (desc->n_used_entries > desc->window.max_entries) {
> -        desc->window.max_entries = desc->n_used_entries;
> +    if (desc->n_used_entries > desc->window_max_entries) {
> +        desc->window_max_entries = desc->n_used_entries;
>      }
> -    rate = desc->window.max_entries * 100 / old_size;
> +    rate = desc->window_max_entries * 100 / old_size;
>
>      if (rate > 70) {
>          new_size = MIN(old_size << 1, 1 << CPU_TLB_DYN_MAX_BITS);
>      } else if (rate < 30 && window_expired) {
> -        size_t ceil = pow2ceil(desc->window.max_entries);
> -        size_t expected_rate = desc->window.max_entries * 100 / ceil;
> +        size_t ceil = pow2ceil(desc->window_max_entries);
> +        size_t expected_rate = desc->window_max_entries * 100 / ceil;
>
>          /*
>           * Avoid undersizing when the max number of entries seen is just below
> @@ -182,7 +182,7 @@ static void tlb_mmu_resize_locked(CPUArchState *env, int mmu_idx)
>
>      if (new_size == old_size) {
>          if (window_expired) {
> -            tlb_window_reset(&desc->window, now, desc->n_used_entries);
> +            tlb_window_reset(desc, now, desc->n_used_entries);
>          }
>          return;
>      }
> @@ -190,7 +190,7 @@ static void tlb_mmu_resize_locked(CPUArchState *env, int mmu_idx)
>      g_free(env->tlb_table[mmu_idx]);
>      g_free(env->iotlb[mmu_idx]);
>
> -    tlb_window_reset(&desc->window, now, 0);
> +    tlb_window_reset(desc, now, 0);
>      /* desc->n_used_entries is cleared by the caller */
>      env->tlb_mask[mmu_idx] = (new_size - 1) << CPU_TLB_ENTRY_BITS;
>      env->tlb_table[mmu_idx] = g_try_new(CPUTLBEntry, new_size);


--
Alex Bennée

Re: [Qemu-devel] [PATCH for-4.1 v2 02/36] tcg: Split out target/arch/cpu-param.h

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:40, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> For all targets, into this new file move TARGET_LONG_BITS,
> TARGET_PAGE_BITS, TARGET_PHYS_ADDR_SPACE_BITS,
> TARGET_VIRT_ADDR_SPACE_BITS, and NB_MMU_MODES.
>
> Include this new file from exec/cpu-defs.h.
>
> This now removes the somewhat odd requirement that target/arch/cpu.h
> defines TARGET_LONG_BITS before including exec/cpu-defs.h, so push the
> bulk of the includes within target/arch/cpu.h to the top.
>
> Acked-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>


Your new headers all need the standard copyright/license
header comment and multiple-include guard.
> diff --git a/target/xtensa/cpu-param.h b/target/xtensa/cpu-param.h
> new file mode 100644
> index 0000000000..000e6026c0
> --- /dev/null
> +++ b/target/xtensa/cpu-param.h
> @@ -0,0 +1,9 @@
> +#define TARGET_LONG_BITS 32
> +#define TARGET_PAGE_BITS 12
> +#define TARGET_PHYS_ADDR_SPACE_BITS 32
> +#ifdef CONFIG_USER_ONLY
> +#define TARGET_VIRT_ADDR_SPACE_BITS 30
> +#else
> +#define TARGET_VIRT_ADDR_SPACE_BITS 32
> +#endif
> +#define NB_MMU_MODES 4
> diff --git a/target/xtensa/cpu.h b/target/xtensa/cpu.h
> index 4d8152682f..93440357b0 100644
> --- a/target/xtensa/cpu.h
> +++ b/target/xtensa/cpu.h
> @@ -28,28 +28,17 @@
>  #ifndef XTENSA_CPU_H
>  #define XTENSA_CPU_H
>
> -#define ALIGNED_ONLY
> -#define TARGET_LONG_BITS 32
> -
> -/* Xtensa processors have a weak memory model */
> -#define TCG_GUEST_DEFAULT_MO      (0)
> -
> -#define CPUArchState struct CPUXtensaState
> -
>  #include "qemu-common.h"
>  #include "cpu-qom.h"
>  #include "exec/cpu-defs.h"
>  #include "xtensa-isa.h"
>
> -#define NB_MMU_MODES 4
> +#define ALIGNED_ONLY

ALIGNED_ONLY is a config thing that tcg.h needs to see -- if
we define it this low in the cpu.h file it seems a bit fragile
whether or not tcg.h will get included by something else
before we get to the definition... (This probably applies
to a few of the other cpu.h files too.)

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 02/36] tcg: Split out target/arch/cpu-param.h

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:40, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> For all targets, into this new file move TARGET_LONG_BITS,
> TARGET_PAGE_BITS, TARGET_PHYS_ADDR_SPACE_BITS,
> TARGET_VIRT_ADDR_SPACE_BITS, and NB_MMU_MODES.
>
> Include this new file from exec/cpu-defs.h.
>
> This now removes the somewhat odd requirement that target/arch/cpu.h
> defines TARGET_LONG_BITS before including exec/cpu-defs.h, so push the
> bulk of the includes within target/arch/cpu.h to the top.
>
> Acked-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>


Your new headers all need the standard copyright/license
header comment and multiple-include guard.
> diff --git a/target/xtensa/cpu-param.h b/target/xtensa/cpu-param.h
> new file mode 100644
> index 0000000000..000e6026c0
> --- /dev/null
> +++ b/target/xtensa/cpu-param.h
> @@ -0,0 +1,9 @@
> +#define TARGET_LONG_BITS 32
> +#define TARGET_PAGE_BITS 12
> +#define TARGET_PHYS_ADDR_SPACE_BITS 32
> +#ifdef CONFIG_USER_ONLY
> +#define TARGET_VIRT_ADDR_SPACE_BITS 30
> +#else
> +#define TARGET_VIRT_ADDR_SPACE_BITS 32
> +#endif
> +#define NB_MMU_MODES 4
> diff --git a/target/xtensa/cpu.h b/target/xtensa/cpu.h
> index 4d8152682f..93440357b0 100644
> --- a/target/xtensa/cpu.h
> +++ b/target/xtensa/cpu.h
> @@ -28,28 +28,17 @@
>  #ifndef XTENSA_CPU_H
>  #define XTENSA_CPU_H
>
> -#define ALIGNED_ONLY
> -#define TARGET_LONG_BITS 32
> -
> -/* Xtensa processors have a weak memory model */
> -#define TCG_GUEST_DEFAULT_MO      (0)
> -
> -#define CPUArchState struct CPUXtensaState
> -
>  #include "qemu-common.h"
>  #include "cpu-qom.h"
>  #include "exec/cpu-defs.h"
>  #include "xtensa-isa.h"
>
> -#define NB_MMU_MODES 4
> +#define ALIGNED_ONLY

ALIGNED_ONLY is a config thing that tcg.h needs to see -- if
we define it this low in the cpu.h file it seems a bit fragile
whether or not tcg.h will get included by something else
before we get to the definition... (This probably applies
to a few of the other cpu.h files too.)

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 03/36] tcg: Create struct CPUTLB

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:52, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Move all softmmu tlb data into this structure.  Arrange the
> members so that we are able to place mask+table together and
> at a smaller absolute offset from ENV.
>
> Acked-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 03/36] tcg: Create struct CPUTLB

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:52, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Move all softmmu tlb data into this structure.  Arrange the
> members so that we are able to place mask+table together and
> at a smaller absolute offset from ENV.
>
> Acked-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 04/36] cpu: Define CPUArchState with typedef

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:35, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> For all targets, do this just before including exec/cpu-all.h.
>
> Acked-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>


Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 04/36] cpu: Define CPUArchState with typedef

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:35, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> For all targets, do this just before including exec/cpu-all.h.
>
> Acked-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>


Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 05/36] cpu: Define ArchCPU

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:47, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> For all targets, do this just before including exec/cpu-all.h.
>
> Acked-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 05/36] cpu: Define ArchCPU

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:47, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> For all targets, do this just before including exec/cpu-all.h.
>
> Acked-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 06/36] cpu: Replace ENV_GET_CPU with env_cpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:41, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Now that we have both ArchCPU and CPUArchState, we can define
> this generically instead of via macro in each target's cpu.h.
>
> Acked-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 06/36] cpu: Replace ENV_GET_CPU with env_cpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:41, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Now that we have both ArchCPU and CPUArchState, we can define
> this generically instead of via macro in each target's cpu.h.
>
> Acked-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 07/36] cpu: Introduce env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:29, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> This will foo_env_get_cpu with a generic definition.

This sentence no verb.

> No changes to the target specific code so far.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 07/36] cpu: Introduce env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:29, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> This will foo_env_get_cpu with a generic definition.

This sentence no verb.

> No changes to the target specific code so far.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 08/36] target/alpha: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:23, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> With exactly one exception, most uses of alpha_env_get_cpu
> were failures to use the more proper, ENV_GET_CPU macro,
> now replaced by env_cpu.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/alpha/cpu.h          | 5 -----
>  linux-user/alpha/cpu_loop.c | 2 +-
>  target/alpha/helper.c       | 8 +++-----
>  target/alpha/sys_helper.c   | 8 ++++----
>  4 files changed, 8 insertions(+), 15 deletions(-)
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 08/36] target/alpha: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:23, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> With exactly one exception, most uses of alpha_env_get_cpu
> were failures to use the more proper, ENV_GET_CPU macro,
> now replaced by env_cpu.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/alpha/cpu.h          | 5 -----
>  linux-user/alpha/cpu_loop.c | 2 +-
>  target/alpha/helper.c       | 8 +++-----
>  target/alpha/sys_helper.c   | 8 ++++----
>  4 files changed, 8 insertions(+), 15 deletions(-)
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 09/36] target/arm: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:51, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Combined uses of CPU(arm_env_get_cpu()) were failures to use
> the more proper, ENV_GET_CPU macro, now replaced by env_cpu.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

(though I imagine that this will need updating when the
M-profile fp patchset hits master.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 09/36] target/arm: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:51, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Combined uses of CPU(arm_env_get_cpu()) were failures to use
> the more proper, ENV_GET_CPU macro, now replaced by env_cpu.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

(though I imagine that this will need updating when the
M-profile fp patchset hits master.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 10/36] target/cris: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:32, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/cris/cpu.h          |  5 -----
>  linux-user/cris/cpu_loop.c |  2 +-
>  target/cris/mmu.c          |  3 +--
>  target/cris/op_helper.c    | 10 +++-------
>  target/cris/translate.c    |  2 +-
>  5 files changed, 6 insertions(+), 16 deletions(-)

> --- a/target/cris/mmu.c
> +++ b/target/cris/mmu.c
> @@ -290,7 +290,6 @@ static int cris_mmu_translate_page(struct cris_mmu_result *res,
>
>  void cris_mmu_flush_pid(CPUCRISState *env, uint32_t pid)
>  {
> -    CRISCPU *cpu = cris_env_get_cpu(env);
>         target_ulong vaddr;
>         unsigned int idx;
>         uint32_t lo, hi;
> @@ -316,7 +315,7 @@ void cris_mmu_flush_pid(CPUCRISState *env, uint32_t pid)
>                                         vaddr = tlb_vpn << TARGET_PAGE_BITS;
>                                         D_LOG("flush pid=%x vaddr=%x\n",
>                                                   pid, vaddr);
> -                    tlb_flush_page(CPU(cpu), vaddr);
> +                    tlb_flush_page(env_cpu(env), vaddr);
>                                 }
>                         }
>                 }

> @@ -143,7 +139,7 @@ void helper_movl_sreg_reg(CPUCRISState *env, uint32_t sreg, uint32_t reg)
>                         D_LOG("tlb flush vaddr=%x v=%d pc=%x\n",
>                                   vaddr, tlb_v, env->pc);
>                         if (tlb_v) {
> -                tlb_flush_page(CPU(cpu), vaddr);
> +                tlb_flush_page(env_cpu(env), vaddr);
>                         }
>                 }
>         }

This file is a mess because it's old-school hard-tabs, but maybe
fix the indent on these lines while you're changing them?

Either way
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 10/36] target/cris: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:32, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/cris/cpu.h          |  5 -----
>  linux-user/cris/cpu_loop.c |  2 +-
>  target/cris/mmu.c          |  3 +--
>  target/cris/op_helper.c    | 10 +++-------
>  target/cris/translate.c    |  2 +-
>  5 files changed, 6 insertions(+), 16 deletions(-)

> --- a/target/cris/mmu.c
> +++ b/target/cris/mmu.c
> @@ -290,7 +290,6 @@ static int cris_mmu_translate_page(struct cris_mmu_result *res,
>
>  void cris_mmu_flush_pid(CPUCRISState *env, uint32_t pid)
>  {
> -    CRISCPU *cpu = cris_env_get_cpu(env);
>         target_ulong vaddr;
>         unsigned int idx;
>         uint32_t lo, hi;
> @@ -316,7 +315,7 @@ void cris_mmu_flush_pid(CPUCRISState *env, uint32_t pid)
>                                         vaddr = tlb_vpn << TARGET_PAGE_BITS;
>                                         D_LOG("flush pid=%x vaddr=%x\n",
>                                                   pid, vaddr);
> -                    tlb_flush_page(CPU(cpu), vaddr);
> +                    tlb_flush_page(env_cpu(env), vaddr);
>                                 }
>                         }
>                 }

> @@ -143,7 +139,7 @@ void helper_movl_sreg_reg(CPUCRISState *env, uint32_t sreg, uint32_t reg)
>                         D_LOG("tlb flush vaddr=%x v=%d pc=%x\n",
>                                   vaddr, tlb_v, env->pc);
>                         if (tlb_v) {
> -                tlb_flush_page(CPU(cpu), vaddr);
> +                tlb_flush_page(env_cpu(env), vaddr);
>                         }
>                 }
>         }

This file is a mess because it's old-school hard-tabs, but maybe
fix the indent on these lines while you're changing them?

Either way
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 11/36] target/hppa: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:26, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Combined uses of CPU(hppa_env_get_cpu()) were failures to use
> the more proper, ENV_GET_CPU macro, now replaced by env_cpu.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/hppa/cpu.h          |  5 -----
>  linux-user/hppa/cpu_loop.c |  2 +-
>  target/hppa/helper.c       |  3 +--
>  target/hppa/int_helper.c   |  4 ++--
>  target/hppa/mem_helper.c   | 10 ++++------
>  target/hppa/op_helper.c    |  8 +++-----
>  6 files changed, 11 insertions(+), 21 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 11/36] target/hppa: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:26, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Combined uses of CPU(hppa_env_get_cpu()) were failures to use
> the more proper, ENV_GET_CPU macro, now replaced by env_cpu.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/hppa/cpu.h          |  5 -----
>  linux-user/hppa/cpu_loop.c |  2 +-
>  target/hppa/helper.c       |  3 +--
>  target/hppa/int_helper.c   |  4 ++--
>  target/hppa/mem_helper.c   | 10 ++++------
>  target/hppa/op_helper.c    |  8 +++-----
>  6 files changed, 11 insertions(+), 21 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 16/36] target/mips: Use env_cpu, env_archcpu

[Philippe Mathieu-Daudé]

On 3/29/19 12:03 AM, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  target/mips/cpu.h                |  5 -----
>  hw/intc/mips_gic.c               |  2 +-
>  hw/mips/mips_int.c               |  2 +-
>  linux-user/mips/cpu_loop.c       |  2 +-
>  target/mips/helper.c             | 15 +++++----------
>  target/mips/op_helper.c          | 25 +++++++++++--------------
>  target/mips/translate.c          |  3 +--
>  target/mips/translate_init.inc.c |  4 +---
>  8 files changed, 21 insertions(+), 37 deletions(-)
> 
> diff --git a/target/mips/cpu.h b/target/mips/cpu.h
> index e7ad81becb..914cc26c21 100644
> --- a/target/mips/cpu.h
> +++ b/target/mips/cpu.h
> @@ -1051,11 +1051,6 @@ struct MIPSCPU {
>      CPUMIPSState env;
>  };
>  
> -static inline MIPSCPU *mips_env_get_cpu(CPUMIPSState *env)
> -{
> -    return container_of(env, MIPSCPU, env);
> -}
> -
>  #define ENV_OFFSET offsetof(MIPSCPU, env)
>  
>  void mips_cpu_list (FILE *f, fprintf_function cpu_fprintf);
> diff --git a/hw/intc/mips_gic.c b/hw/intc/mips_gic.c
> index 15e6e40f9f..8f509493ea 100644
> --- a/hw/intc/mips_gic.c
> +++ b/hw/intc/mips_gic.c
> @@ -44,7 +44,7 @@ static void mips_gic_set_vp_irq(MIPSGICState *gic, int vp, int pin)
>                        GIC_VP_MASK_CMP_SHF;
>      }
>      if (kvm_enabled())  {
> -        kvm_mips_set_ipi_interrupt(mips_env_get_cpu(gic->vps[vp].env),
> +        kvm_mips_set_ipi_interrupt(env_archcpu(gic->vps[vp].env),
>                                     pin + GIC_CPU_PIN_OFFSET,
>                                     ored_level);
>      } else {
> diff --git a/hw/mips/mips_int.c b/hw/mips/mips_int.c
> index 5ddeb15848..f899f6ceb3 100644
> --- a/hw/mips/mips_int.c
> +++ b/hw/mips/mips_int.c
> @@ -76,7 +76,7 @@ void cpu_mips_irq_init_cpu(MIPSCPU *cpu)
>      qemu_irq *qi;
>      int i;
>  
> -    qi = qemu_allocate_irqs(cpu_mips_irq_request, mips_env_get_cpu(env), 8);
> +    qi = qemu_allocate_irqs(cpu_mips_irq_request, env_archcpu(env), 8);
>      for (i = 0; i < 8; i++) {
>          env->irq[i] = qi[i];
>      }
> diff --git a/linux-user/mips/cpu_loop.c b/linux-user/mips/cpu_loop.c
> index 828137cd84..ac6c6d1504 100644
> --- a/linux-user/mips/cpu_loop.c
> +++ b/linux-user/mips/cpu_loop.c
> @@ -425,7 +425,7 @@ static int do_break(CPUMIPSState *env, target_siginfo_t *info,
>  
>  void cpu_loop(CPUMIPSState *env)
>  {
> -    CPUState *cs = CPU(mips_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>      target_siginfo_t info;
>      int trapnr;
>      abi_long ret;
> diff --git a/target/mips/helper.c b/target/mips/helper.c
> index c44cdca3b5..1fc0a4ce4b 100644
> --- a/target/mips/helper.c
> +++ b/target/mips/helper.c
> @@ -336,10 +336,8 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical,
>  
>  void cpu_mips_tlb_flush(CPUMIPSState *env)
>  {
> -    MIPSCPU *cpu = mips_env_get_cpu(env);
> -
>      /* Flush qemu's TLB and discard all shadowed entries.  */
> -    tlb_flush(CPU(cpu));
> +    tlb_flush(env_cpu(env));
>      env->tlb->tlb_in_use = env->tlb->nb_tlb;
>  }
>  
> @@ -401,7 +399,7 @@ void cpu_mips_store_status(CPUMIPSState *env, target_ulong val)
>  #if defined(TARGET_MIPS64)
>      if ((env->CP0_Status ^ old) & (old & (7 << CP0St_UX))) {
>          /* Access to at least one of the 64-bit segments has been disabled */
> -        tlb_flush(CPU(mips_env_get_cpu(env)));
> +        tlb_flush(env_cpu(env));
>      }
>  #endif
>      if (env->CP0_Config3 & (1 << CP0C3_MT)) {
> @@ -446,7 +444,7 @@ void cpu_mips_store_cause(CPUMIPSState *env, target_ulong val)
>  static void raise_mmu_exception(CPUMIPSState *env, target_ulong address,
>                                  int rw, int tlb_error)
>  {
> -    CPUState *cs = CPU(mips_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>      int exception = 0, error_code = 0;
>  
>      if (rw == MMU_INST_FETCH) {
> @@ -1400,8 +1398,7 @@ bool mips_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>  #if !defined(CONFIG_USER_ONLY)
>  void r4k_invalidate_tlb (CPUMIPSState *env, int idx, int use_extra)
>  {
> -    MIPSCPU *cpu = mips_env_get_cpu(env);
> -    CPUState *cs;
> +    CPUState *cs = env_cpu(env);
>      r4k_tlb_t *tlb;
>      target_ulong addr;
>      target_ulong end;
> @@ -1427,7 +1424,6 @@ void r4k_invalidate_tlb (CPUMIPSState *env, int idx, int use_extra)
>      /* 1k pages are not supported. */
>      mask = tlb->PageMask | ~(TARGET_PAGE_MASK << 1);
>      if (tlb->V0) {
> -        cs = CPU(cpu);
>          addr = tlb->VPN & ~mask;
>  #if defined(TARGET_MIPS64)
>          if (addr >= (0xFFFFFFFF80000000ULL & env->SEGMask)) {
> @@ -1441,7 +1437,6 @@ void r4k_invalidate_tlb (CPUMIPSState *env, int idx, int use_extra)
>          }
>      }
>      if (tlb->V1) {
> -        cs = CPU(cpu);
>          addr = (tlb->VPN & ~mask) | ((mask >> 1) + 1);
>  #if defined(TARGET_MIPS64)
>          if (addr >= (0xFFFFFFFF80000000ULL & env->SEGMask)) {
> @@ -1462,7 +1457,7 @@ void QEMU_NORETURN do_raise_exception_err(CPUMIPSState *env,
>                                            int error_code,
>                                            uintptr_t pc)
>  {
> -    CPUState *cs = CPU(mips_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      qemu_log_mask(CPU_LOG_INT, "%s: %d %d\n",
>                    __func__, exception, error_code);
> diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
> index 0f272a5b93..0705e8c686 100644
> --- a/target/mips/op_helper.c
> +++ b/target/mips/op_helper.c
> @@ -350,7 +350,7 @@ static inline hwaddr do_translate_address(CPUMIPSState *env,
>                                                        int rw, uintptr_t retaddr)
>  {
>      hwaddr paddr;
> -    CPUState *cs = CPU(mips_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      paddr = cpu_mips_translate_address(env, address, rw);
>  
> @@ -699,7 +699,7 @@ static CPUMIPSState *mips_cpu_map_tc(CPUMIPSState *env, int *tc)
>          return env;
>      }
>  
> -    cs = CPU(mips_env_get_cpu(env));
> +    cs = env_cpu(env);
>      vpe_idx = tc_idx / cs->nr_threads;
>      *tc = tc_idx % cs->nr_threads;
>      other_cs = qemu_get_cpu(vpe_idx);
> @@ -1298,7 +1298,7 @@ void helper_mttc0_tcrestart(CPUMIPSState *env, target_ulong arg1)
>  
>  void helper_mtc0_tchalt(CPUMIPSState *env, target_ulong arg1)
>  {
> -    MIPSCPU *cpu = mips_env_get_cpu(env);
> +    MIPSCPU *cpu = env_archcpu(env);
>  
>      env->active_tc.CP0_TCHalt = arg1 & 0x1;
>  
> @@ -1314,7 +1314,7 @@ void helper_mttc0_tchalt(CPUMIPSState *env, target_ulong arg1)
>  {
>      int other_tc = env->CP0_VPEControl & (0xff << CP0VPECo_TargTC);
>      CPUMIPSState *other = mips_cpu_map_tc(env, &other_tc);
> -    MIPSCPU *other_cpu = mips_env_get_cpu(other);
> +    MIPSCPU *other_cpu = env_archcpu(other);
>  
>      // TODO: Halt TC / Restart (if allocated+active) TC.
>  
> @@ -1427,7 +1427,7 @@ void helper_mtc0_pagegrain(CPUMIPSState *env, target_ulong arg1)
>  
>  void helper_mtc0_segctl0(CPUMIPSState *env, target_ulong arg1)
>  {
> -    CPUState *cs = CPU(mips_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      env->CP0_SegCtl0 = arg1 & CP0SC0_MASK;
>      tlb_flush(cs);
> @@ -1435,7 +1435,7 @@ void helper_mtc0_segctl0(CPUMIPSState *env, target_ulong arg1)
>  
>  void helper_mtc0_segctl1(CPUMIPSState *env, target_ulong arg1)
>  {
> -    CPUState *cs = CPU(mips_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      env->CP0_SegCtl1 = arg1 & CP0SC1_MASK;
>      tlb_flush(cs);
> @@ -1443,7 +1443,7 @@ void helper_mtc0_segctl1(CPUMIPSState *env, target_ulong arg1)
>  
>  void helper_mtc0_segctl2(CPUMIPSState *env, target_ulong arg1)
>  {
> -    CPUState *cs = CPU(mips_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      env->CP0_SegCtl2 = arg1 & CP0SC2_MASK;
>      tlb_flush(cs);
> @@ -1666,7 +1666,7 @@ void helper_mtc0_entryhi(CPUMIPSState *env, target_ulong arg1)
>      /* If the ASID changes, flush qemu's TLB.  */
>      if ((old & env->CP0_EntryHi_ASID_mask) !=
>          (val & env->CP0_EntryHi_ASID_mask)) {
> -        tlb_flush(CPU(mips_env_get_cpu(env)));
> +        tlb_flush(env_cpu(env));
>      }
>  }
>  
> @@ -1686,7 +1686,6 @@ void helper_mtc0_compare(CPUMIPSState *env, target_ulong arg1)
>  
>  void helper_mtc0_status(CPUMIPSState *env, target_ulong arg1)
>  {
> -    MIPSCPU *cpu = mips_env_get_cpu(env);
>      uint32_t val, old;
>  
>      old = env->CP0_Status;
> @@ -1706,7 +1705,7 @@ void helper_mtc0_status(CPUMIPSState *env, target_ulong arg1)
>          case MIPS_HFLAG_SM: qemu_log(", SM\n"); break;
>          case MIPS_HFLAG_KM: qemu_log("\n"); break;
>          default:
> -            cpu_abort(CPU(cpu), "Invalid MMU mode!\n");
> +            cpu_abort(env_cpu(env), "Invalid MMU mode!\n");
>              break;
>          }
>      }
> @@ -2485,8 +2484,6 @@ static void debug_pre_eret(CPUMIPSState *env)
>  
>  static void debug_post_eret(CPUMIPSState *env)
>  {
> -    MIPSCPU *cpu = mips_env_get_cpu(env);
> -
>      if (qemu_loglevel_mask(CPU_LOG_EXEC)) {
>          qemu_log("  =>  PC " TARGET_FMT_lx " EPC " TARGET_FMT_lx,
>                  env->active_tc.PC, env->CP0_EPC);
> @@ -2502,7 +2499,7 @@ static void debug_post_eret(CPUMIPSState *env)
>          case MIPS_HFLAG_SM: qemu_log(", SM\n"); break;
>          case MIPS_HFLAG_KM: qemu_log("\n"); break;
>          default:
> -            cpu_abort(CPU(cpu), "Invalid MMU mode!\n");
> +            cpu_abort(env_cpu(env), "Invalid MMU mode!\n");
>              break;
>          }
>      }
> @@ -2633,7 +2630,7 @@ void helper_pmon(CPUMIPSState *env, int function)
>  
>  void helper_wait(CPUMIPSState *env)
>  {
> -    CPUState *cs = CPU(mips_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      cs->halted = 1;
>      cpu_reset_interrupt(cs, CPU_INTERRUPT_WAKE);
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 364bd6dc4f..7ace8c3b96 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -29911,8 +29911,7 @@ void cpu_set_exception_base(int vp_index, target_ulong address)
>  
>  void cpu_state_reset(CPUMIPSState *env)
>  {
> -    MIPSCPU *cpu = mips_env_get_cpu(env);
> -    CPUState *cs = CPU(cpu);
> +    CPUState *cs = env_cpu(env);
>  
>      /* Reset registers to their default values */
>      env->CP0_PRid = env->cpu_model->CP0_PRid;
> diff --git a/target/mips/translate_init.inc.c b/target/mips/translate_init.inc.c
> index bf559aff08..50586a01a2 100644
> --- a/target/mips/translate_init.inc.c
> +++ b/target/mips/translate_init.inc.c
> @@ -872,8 +872,6 @@ static void r4k_mmu_init (CPUMIPSState *env, const mips_def_t *def)
>  
>  static void mmu_init (CPUMIPSState *env, const mips_def_t *def)
>  {
> -    MIPSCPU *cpu = mips_env_get_cpu(env);
> -
>      env->tlb = g_malloc0(sizeof(CPUMIPSTLBContext));
>  
>      switch (def->mmu_type) {
> @@ -890,7 +888,7 @@ static void mmu_init (CPUMIPSState *env, const mips_def_t *def)
>          case MMU_TYPE_R6000:
>          case MMU_TYPE_R8000:
>          default:
> -            cpu_abort(CPU(cpu), "MMU type not supported\n");
> +            cpu_abort(env_cpu(env), "MMU type not supported\n");
>      }
>  }
>  #endif /* CONFIG_USER_ONLY */
> 

Re: [Qemu-devel] [PATCH for-4.1 v2 16/36] target/mips: Use env_cpu, env_archcpu

[Philippe Mathieu-Daudé]

On 3/29/19 12:03 AM, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  target/mips/cpu.h                |  5 -----
>  hw/intc/mips_gic.c               |  2 +-
>  hw/mips/mips_int.c               |  2 +-
>  linux-user/mips/cpu_loop.c       |  2 +-
>  target/mips/helper.c             | 15 +++++----------
>  target/mips/op_helper.c          | 25 +++++++++++--------------
>  target/mips/translate.c          |  3 +--
>  target/mips/translate_init.inc.c |  4 +---
>  8 files changed, 21 insertions(+), 37 deletions(-)
> 
> diff --git a/target/mips/cpu.h b/target/mips/cpu.h
> index e7ad81becb..914cc26c21 100644
> --- a/target/mips/cpu.h
> +++ b/target/mips/cpu.h
> @@ -1051,11 +1051,6 @@ struct MIPSCPU {
>      CPUMIPSState env;
>  };
>  
> -static inline MIPSCPU *mips_env_get_cpu(CPUMIPSState *env)
> -{
> -    return container_of(env, MIPSCPU, env);
> -}
> -
>  #define ENV_OFFSET offsetof(MIPSCPU, env)
>  
>  void mips_cpu_list (FILE *f, fprintf_function cpu_fprintf);
> diff --git a/hw/intc/mips_gic.c b/hw/intc/mips_gic.c
> index 15e6e40f9f..8f509493ea 100644
> --- a/hw/intc/mips_gic.c
> +++ b/hw/intc/mips_gic.c
> @@ -44,7 +44,7 @@ static void mips_gic_set_vp_irq(MIPSGICState *gic, int vp, int pin)
>                        GIC_VP_MASK_CMP_SHF;
>      }
>      if (kvm_enabled())  {
> -        kvm_mips_set_ipi_interrupt(mips_env_get_cpu(gic->vps[vp].env),
> +        kvm_mips_set_ipi_interrupt(env_archcpu(gic->vps[vp].env),
>                                     pin + GIC_CPU_PIN_OFFSET,
>                                     ored_level);
>      } else {
> diff --git a/hw/mips/mips_int.c b/hw/mips/mips_int.c
> index 5ddeb15848..f899f6ceb3 100644
> --- a/hw/mips/mips_int.c
> +++ b/hw/mips/mips_int.c
> @@ -76,7 +76,7 @@ void cpu_mips_irq_init_cpu(MIPSCPU *cpu)
>      qemu_irq *qi;
>      int i;
>  
> -    qi = qemu_allocate_irqs(cpu_mips_irq_request, mips_env_get_cpu(env), 8);
> +    qi = qemu_allocate_irqs(cpu_mips_irq_request, env_archcpu(env), 8);
>      for (i = 0; i < 8; i++) {
>          env->irq[i] = qi[i];
>      }
> diff --git a/linux-user/mips/cpu_loop.c b/linux-user/mips/cpu_loop.c
> index 828137cd84..ac6c6d1504 100644
> --- a/linux-user/mips/cpu_loop.c
> +++ b/linux-user/mips/cpu_loop.c
> @@ -425,7 +425,7 @@ static int do_break(CPUMIPSState *env, target_siginfo_t *info,
>  
>  void cpu_loop(CPUMIPSState *env)
>  {
> -    CPUState *cs = CPU(mips_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>      target_siginfo_t info;
>      int trapnr;
>      abi_long ret;
> diff --git a/target/mips/helper.c b/target/mips/helper.c
> index c44cdca3b5..1fc0a4ce4b 100644
> --- a/target/mips/helper.c
> +++ b/target/mips/helper.c
> @@ -336,10 +336,8 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical,
>  
>  void cpu_mips_tlb_flush(CPUMIPSState *env)
>  {
> -    MIPSCPU *cpu = mips_env_get_cpu(env);
> -
>      /* Flush qemu's TLB and discard all shadowed entries.  */
> -    tlb_flush(CPU(cpu));
> +    tlb_flush(env_cpu(env));
>      env->tlb->tlb_in_use = env->tlb->nb_tlb;
>  }
>  
> @@ -401,7 +399,7 @@ void cpu_mips_store_status(CPUMIPSState *env, target_ulong val)
>  #if defined(TARGET_MIPS64)
>      if ((env->CP0_Status ^ old) & (old & (7 << CP0St_UX))) {
>          /* Access to at least one of the 64-bit segments has been disabled */
> -        tlb_flush(CPU(mips_env_get_cpu(env)));
> +        tlb_flush(env_cpu(env));
>      }
>  #endif
>      if (env->CP0_Config3 & (1 << CP0C3_MT)) {
> @@ -446,7 +444,7 @@ void cpu_mips_store_cause(CPUMIPSState *env, target_ulong val)
>  static void raise_mmu_exception(CPUMIPSState *env, target_ulong address,
>                                  int rw, int tlb_error)
>  {
> -    CPUState *cs = CPU(mips_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>      int exception = 0, error_code = 0;
>  
>      if (rw == MMU_INST_FETCH) {
> @@ -1400,8 +1398,7 @@ bool mips_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>  #if !defined(CONFIG_USER_ONLY)
>  void r4k_invalidate_tlb (CPUMIPSState *env, int idx, int use_extra)
>  {
> -    MIPSCPU *cpu = mips_env_get_cpu(env);
> -    CPUState *cs;
> +    CPUState *cs = env_cpu(env);
>      r4k_tlb_t *tlb;
>      target_ulong addr;
>      target_ulong end;
> @@ -1427,7 +1424,6 @@ void r4k_invalidate_tlb (CPUMIPSState *env, int idx, int use_extra)
>      /* 1k pages are not supported. */
>      mask = tlb->PageMask | ~(TARGET_PAGE_MASK << 1);
>      if (tlb->V0) {
> -        cs = CPU(cpu);
>          addr = tlb->VPN & ~mask;
>  #if defined(TARGET_MIPS64)
>          if (addr >= (0xFFFFFFFF80000000ULL & env->SEGMask)) {
> @@ -1441,7 +1437,6 @@ void r4k_invalidate_tlb (CPUMIPSState *env, int idx, int use_extra)
>          }
>      }
>      if (tlb->V1) {
> -        cs = CPU(cpu);
>          addr = (tlb->VPN & ~mask) | ((mask >> 1) + 1);
>  #if defined(TARGET_MIPS64)
>          if (addr >= (0xFFFFFFFF80000000ULL & env->SEGMask)) {
> @@ -1462,7 +1457,7 @@ void QEMU_NORETURN do_raise_exception_err(CPUMIPSState *env,
>                                            int error_code,
>                                            uintptr_t pc)
>  {
> -    CPUState *cs = CPU(mips_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      qemu_log_mask(CPU_LOG_INT, "%s: %d %d\n",
>                    __func__, exception, error_code);
> diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
> index 0f272a5b93..0705e8c686 100644
> --- a/target/mips/op_helper.c
> +++ b/target/mips/op_helper.c
> @@ -350,7 +350,7 @@ static inline hwaddr do_translate_address(CPUMIPSState *env,
>                                                        int rw, uintptr_t retaddr)
>  {
>      hwaddr paddr;
> -    CPUState *cs = CPU(mips_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      paddr = cpu_mips_translate_address(env, address, rw);
>  
> @@ -699,7 +699,7 @@ static CPUMIPSState *mips_cpu_map_tc(CPUMIPSState *env, int *tc)
>          return env;
>      }
>  
> -    cs = CPU(mips_env_get_cpu(env));
> +    cs = env_cpu(env);
>      vpe_idx = tc_idx / cs->nr_threads;
>      *tc = tc_idx % cs->nr_threads;
>      other_cs = qemu_get_cpu(vpe_idx);
> @@ -1298,7 +1298,7 @@ void helper_mttc0_tcrestart(CPUMIPSState *env, target_ulong arg1)
>  
>  void helper_mtc0_tchalt(CPUMIPSState *env, target_ulong arg1)
>  {
> -    MIPSCPU *cpu = mips_env_get_cpu(env);
> +    MIPSCPU *cpu = env_archcpu(env);
>  
>      env->active_tc.CP0_TCHalt = arg1 & 0x1;
>  
> @@ -1314,7 +1314,7 @@ void helper_mttc0_tchalt(CPUMIPSState *env, target_ulong arg1)
>  {
>      int other_tc = env->CP0_VPEControl & (0xff << CP0VPECo_TargTC);
>      CPUMIPSState *other = mips_cpu_map_tc(env, &other_tc);
> -    MIPSCPU *other_cpu = mips_env_get_cpu(other);
> +    MIPSCPU *other_cpu = env_archcpu(other);
>  
>      // TODO: Halt TC / Restart (if allocated+active) TC.
>  
> @@ -1427,7 +1427,7 @@ void helper_mtc0_pagegrain(CPUMIPSState *env, target_ulong arg1)
>  
>  void helper_mtc0_segctl0(CPUMIPSState *env, target_ulong arg1)
>  {
> -    CPUState *cs = CPU(mips_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      env->CP0_SegCtl0 = arg1 & CP0SC0_MASK;
>      tlb_flush(cs);
> @@ -1435,7 +1435,7 @@ void helper_mtc0_segctl0(CPUMIPSState *env, target_ulong arg1)
>  
>  void helper_mtc0_segctl1(CPUMIPSState *env, target_ulong arg1)
>  {
> -    CPUState *cs = CPU(mips_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      env->CP0_SegCtl1 = arg1 & CP0SC1_MASK;
>      tlb_flush(cs);
> @@ -1443,7 +1443,7 @@ void helper_mtc0_segctl1(CPUMIPSState *env, target_ulong arg1)
>  
>  void helper_mtc0_segctl2(CPUMIPSState *env, target_ulong arg1)
>  {
> -    CPUState *cs = CPU(mips_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      env->CP0_SegCtl2 = arg1 & CP0SC2_MASK;
>      tlb_flush(cs);
> @@ -1666,7 +1666,7 @@ void helper_mtc0_entryhi(CPUMIPSState *env, target_ulong arg1)
>      /* If the ASID changes, flush qemu's TLB.  */
>      if ((old & env->CP0_EntryHi_ASID_mask) !=
>          (val & env->CP0_EntryHi_ASID_mask)) {
> -        tlb_flush(CPU(mips_env_get_cpu(env)));
> +        tlb_flush(env_cpu(env));
>      }
>  }
>  
> @@ -1686,7 +1686,6 @@ void helper_mtc0_compare(CPUMIPSState *env, target_ulong arg1)
>  
>  void helper_mtc0_status(CPUMIPSState *env, target_ulong arg1)
>  {
> -    MIPSCPU *cpu = mips_env_get_cpu(env);
>      uint32_t val, old;
>  
>      old = env->CP0_Status;
> @@ -1706,7 +1705,7 @@ void helper_mtc0_status(CPUMIPSState *env, target_ulong arg1)
>          case MIPS_HFLAG_SM: qemu_log(", SM\n"); break;
>          case MIPS_HFLAG_KM: qemu_log("\n"); break;
>          default:
> -            cpu_abort(CPU(cpu), "Invalid MMU mode!\n");
> +            cpu_abort(env_cpu(env), "Invalid MMU mode!\n");
>              break;
>          }
>      }
> @@ -2485,8 +2484,6 @@ static void debug_pre_eret(CPUMIPSState *env)
>  
>  static void debug_post_eret(CPUMIPSState *env)
>  {
> -    MIPSCPU *cpu = mips_env_get_cpu(env);
> -
>      if (qemu_loglevel_mask(CPU_LOG_EXEC)) {
>          qemu_log("  =>  PC " TARGET_FMT_lx " EPC " TARGET_FMT_lx,
>                  env->active_tc.PC, env->CP0_EPC);
> @@ -2502,7 +2499,7 @@ static void debug_post_eret(CPUMIPSState *env)
>          case MIPS_HFLAG_SM: qemu_log(", SM\n"); break;
>          case MIPS_HFLAG_KM: qemu_log("\n"); break;
>          default:
> -            cpu_abort(CPU(cpu), "Invalid MMU mode!\n");
> +            cpu_abort(env_cpu(env), "Invalid MMU mode!\n");
>              break;
>          }
>      }
> @@ -2633,7 +2630,7 @@ void helper_pmon(CPUMIPSState *env, int function)
>  
>  void helper_wait(CPUMIPSState *env)
>  {
> -    CPUState *cs = CPU(mips_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      cs->halted = 1;
>      cpu_reset_interrupt(cs, CPU_INTERRUPT_WAKE);
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 364bd6dc4f..7ace8c3b96 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -29911,8 +29911,7 @@ void cpu_set_exception_base(int vp_index, target_ulong address)
>  
>  void cpu_state_reset(CPUMIPSState *env)
>  {
> -    MIPSCPU *cpu = mips_env_get_cpu(env);
> -    CPUState *cs = CPU(cpu);
> +    CPUState *cs = env_cpu(env);
>  
>      /* Reset registers to their default values */
>      env->CP0_PRid = env->cpu_model->CP0_PRid;
> diff --git a/target/mips/translate_init.inc.c b/target/mips/translate_init.inc.c
> index bf559aff08..50586a01a2 100644
> --- a/target/mips/translate_init.inc.c
> +++ b/target/mips/translate_init.inc.c
> @@ -872,8 +872,6 @@ static void r4k_mmu_init (CPUMIPSState *env, const mips_def_t *def)
>  
>  static void mmu_init (CPUMIPSState *env, const mips_def_t *def)
>  {
> -    MIPSCPU *cpu = mips_env_get_cpu(env);
> -
>      env->tlb = g_malloc0(sizeof(CPUMIPSTLBContext));
>  
>      switch (def->mmu_type) {
> @@ -890,7 +888,7 @@ static void mmu_init (CPUMIPSState *env, const mips_def_t *def)
>          case MMU_TYPE_R6000:
>          case MMU_TYPE_R8000:
>          default:
> -            cpu_abort(CPU(cpu), "MMU type not supported\n");
> +            cpu_abort(env_cpu(env), "MMU type not supported\n");
>      }
>  }
>  #endif /* CONFIG_USER_ONLY */
> 

Re: [Qemu-devel] [PATCH for-4.1 v2 12/36] target/i386: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:48, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Combined uses of CPU(x86_env_get_cpu()) were failures to use
> the more proper, ENV_GET_CPU macro, now replaced by env_cpu.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 12/36] target/i386: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:48, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Combined uses of CPU(x86_env_get_cpu()) were failures to use
> the more proper, ENV_GET_CPU macro, now replaced by env_cpu.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 13/36] target/lm32: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:49, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/lm32/cpu.h       |  5 -----
>  target/lm32/helper.c    | 19 ++++++-------------
>  target/lm32/op_helper.c |  6 +++---
>  target/lm32/translate.c |  2 +-
>  4 files changed, 10 insertions(+), 22 deletions(-)
>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 13/36] target/lm32: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:49, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/lm32/cpu.h       |  5 -----
>  target/lm32/helper.c    | 19 ++++++-------------
>  target/lm32/op_helper.c |  6 +++---
>  target/lm32/translate.c |  2 +-
>  4 files changed, 10 insertions(+), 22 deletions(-)
>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 14/36] target/m68k: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:45, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 14/36] target/m68k: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:45, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 15/36] target/microblaze: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:41, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Mention in the commit message why we needed to move the
definition of cpu_mmu_index() ?

Anyway
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 15/36] target/microblaze: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:41, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Mention in the commit message why we needed to move the
definition of cpu_mmu_index() ?

Anyway
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 17/36] target/moxie: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:38, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/moxie/cpu.h       | 5 -----
>  target/moxie/helper.c    | 6 +++---
>  target/moxie/translate.c | 2 +-
>  3 files changed, 4 insertions(+), 9 deletions(-)
>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 17/36] target/moxie: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:38, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/moxie/cpu.h       | 5 -----
>  target/moxie/helper.c    | 6 +++---
>  target/moxie/translate.c | 2 +-
>  3 files changed, 4 insertions(+), 9 deletions(-)
>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 18/36] target/nios2: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:38, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/nios2/cpu.h |  5 -----
>  hw/nios2/cpu_pic.c |  5 +----
>  target/nios2/mmu.c | 10 +++++-----
>  3 files changed, 6 insertions(+), 14 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 18/36] target/nios2: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:38, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/nios2/cpu.h |  5 -----
>  hw/nios2/cpu_pic.c |  5 +----
>  target/nios2/mmu.c | 10 +++++-----
>  3 files changed, 6 insertions(+), 14 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 23/36] target/sh4: Use env_cpu, env_archcpu

[Philippe Mathieu-Daudé]

On 3/29/19 12:03 AM, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/sh4/cpu.h          |  5 -----
>  linux-user/sh4/cpu_loop.c |  2 +-
>  target/sh4/helper.c       | 26 ++++++++++++--------------
>  target/sh4/op_helper.c    |  9 +++------
>  4 files changed, 16 insertions(+), 26 deletions(-)
> 
> diff --git a/target/sh4/cpu.h b/target/sh4/cpu.h
> index 95094a517a..1f94e7bf7b 100644
> --- a/target/sh4/cpu.h
> +++ b/target/sh4/cpu.h
> @@ -207,11 +207,6 @@ struct SuperHCPU {
>      CPUSH4State env;
>  };
>  
> -static inline SuperHCPU *sh_env_get_cpu(CPUSH4State *env)
> -{
> -    return container_of(env, SuperHCPU, env);
> -}
> -
>  #define ENV_OFFSET offsetof(SuperHCPU, env)
>  
>  void superh_cpu_do_interrupt(CPUState *cpu);
> diff --git a/linux-user/sh4/cpu_loop.c b/linux-user/sh4/cpu_loop.c
> index 47e54b9b61..677c5a461c 100644
> --- a/linux-user/sh4/cpu_loop.c
> +++ b/linux-user/sh4/cpu_loop.c
> @@ -23,7 +23,7 @@
>  
>  void cpu_loop(CPUSH4State *env)
>  {
> -    CPUState *cs = CPU(sh_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>      int trapnr, ret;
>      target_siginfo_t info;
>  
> diff --git a/target/sh4/helper.c b/target/sh4/helper.c
> index 2ff0cf4060..5240da715e 100644
> --- a/target/sh4/helper.c
> +++ b/target/sh4/helper.c
> @@ -238,8 +238,6 @@ static void update_itlb_use(CPUSH4State * env, int itlbnb)
>  
>  static int itlb_replacement(CPUSH4State * env)
>  {
> -    SuperHCPU *cpu = sh_env_get_cpu(env);
> -
>      if ((env->mmucr & 0xe0000000) == 0xe0000000) {
>  	return 0;
>      }
> @@ -252,7 +250,7 @@ static int itlb_replacement(CPUSH4State * env)
>      if ((env->mmucr & 0x2c000000) == 0x00000000) {
>  	return 3;
>      }
> -    cpu_abort(CPU(cpu), "Unhandled itlb_replacement");
> +    cpu_abort(env_cpu(env), "Unhandled itlb_replacement");
>  }
>  
>  /* Find the corresponding entry in the right TLB
> @@ -308,7 +306,7 @@ static int copy_utlb_entry_itlb(CPUSH4State *env, int utlb)
>      itlb = itlb_replacement(env);
>      ientry = &env->itlb[itlb];
>      if (ientry->v) {
> -        tlb_flush_page(CPU(sh_env_get_cpu(env)), ientry->vpn << 10);
> +        tlb_flush_page(env_cpu(env), ientry->vpn << 10);
>      }
>      *ientry = env->utlb[utlb];
>      update_itlb_use(env, itlb);
> @@ -533,14 +531,14 @@ hwaddr superh_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
>  
>  void cpu_load_tlb(CPUSH4State * env)
>  {
> -    SuperHCPU *cpu = sh_env_get_cpu(env);
> +    CPUState *cs = env_cpu(env);
>      int n = cpu_mmucr_urc(env->mmucr);
>      tlb_t * entry = &env->utlb[n];
>  
>      if (entry->v) {
>          /* Overwriting valid entry in utlb. */
>          target_ulong address = entry->vpn << 10;
> -        tlb_flush_page(CPU(cpu), address);
> +        tlb_flush_page(cs, address);
>      }
>  
>      /* Take values into cpu status from registers. */
> @@ -563,7 +561,7 @@ void cpu_load_tlb(CPUSH4State * env)
>          entry->size = 1024 * 1024; /* 1M */
>          break;
>      default:
> -        cpu_abort(CPU(cpu), "Unhandled load_tlb");
> +        cpu_abort(cs, "Unhandled load_tlb");
>          break;
>      }
>      entry->sh   = (uint8_t)cpu_ptel_sh(env->ptel);
> @@ -590,7 +588,7 @@ void cpu_load_tlb(CPUSH4State * env)
>          entry->v = 0;
>      }
>  
> -    tlb_flush(CPU(sh_env_get_cpu(s)));
> +    tlb_flush(env_cpu(s));
>  }
>  
>  uint32_t cpu_sh4_read_mmaped_itlb_addr(CPUSH4State *s,
> @@ -616,7 +614,7 @@ void cpu_sh4_write_mmaped_itlb_addr(CPUSH4State *s, hwaddr addr,
>      if (entry->v) {
>          /* Overwriting valid entry in itlb. */
>          target_ulong address = entry->vpn << 10;
> -        tlb_flush_page(CPU(sh_env_get_cpu(s)), address);
> +        tlb_flush_page(env_cpu(s), address);
>      }
>      entry->asid = asid;
>      entry->vpn = vpn;
> @@ -658,7 +656,7 @@ void cpu_sh4_write_mmaped_itlb_data(CPUSH4State *s, hwaddr addr,
>          if (entry->v) {
>              /* Overwriting valid entry in utlb. */
>              target_ulong address = entry->vpn << 10;
> -            tlb_flush_page(CPU(sh_env_get_cpu(s)), address);
> +            tlb_flush_page(env_cpu(s), address);
>          }
>          entry->ppn = (mem_value & 0x1ffffc00) >> 10;
>          entry->v   = (mem_value & 0x00000100) >> 8;
> @@ -711,7 +709,7 @@ void cpu_sh4_write_mmaped_utlb_addr(CPUSH4State *s, hwaddr addr,
>              if (entry->vpn == vpn
>                  && (!use_asid || entry->asid == asid || entry->sh)) {
>  	        if (utlb_match_entry) {
> -                    CPUState *cs = CPU(sh_env_get_cpu(s));
> +                    CPUState *cs = env_cpu(s);
>  
>  		    /* Multiple TLB Exception */
>                      cs->exception_index = 0x140;
> @@ -743,14 +741,14 @@ void cpu_sh4_write_mmaped_utlb_addr(CPUSH4State *s, hwaddr addr,
>  	}
>  
>          if (needs_tlb_flush) {
> -            tlb_flush_page(CPU(sh_env_get_cpu(s)), vpn << 10);
> +            tlb_flush_page(env_cpu(s), vpn << 10);
>          }
>          
>      } else {
>          int index = (addr & 0x00003f00) >> 8;
>          tlb_t * entry = &s->utlb[index];
>  	if (entry->v) {
> -            CPUState *cs = CPU(sh_env_get_cpu(s));
> +            CPUState *cs = env_cpu(s);
>  
>  	    /* Overwriting valid entry in utlb. */
>              target_ulong address = entry->vpn << 10;
> @@ -805,7 +803,7 @@ void cpu_sh4_write_mmaped_utlb_data(CPUSH4State *s, hwaddr addr,
>          if (entry->v) {
>              /* Overwriting valid entry in utlb. */
>              target_ulong address = entry->vpn << 10;
> -            tlb_flush_page(CPU(sh_env_get_cpu(s)), address);
> +            tlb_flush_page(env_cpu(s), address);
>          }
>          entry->ppn = (mem_value & 0x1ffffc00) >> 10;
>          entry->v   = (mem_value & 0x00000100) >> 8;
> diff --git a/target/sh4/op_helper.c b/target/sh4/op_helper.c
> index 12fba6fc78..11cb68cc1c 100644
> --- a/target/sh4/op_helper.c
> +++ b/target/sh4/op_helper.c
> @@ -58,10 +58,7 @@ void tlb_fill(CPUState *cs, target_ulong addr, int size,
>  void helper_ldtlb(CPUSH4State *env)
>  {
>  #ifdef CONFIG_USER_ONLY
> -    SuperHCPU *cpu = sh_env_get_cpu(env);
> -
> -    /* XXXXX */

Left-over from commit 43dc2a645e0, OK.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> -    cpu_abort(CPU(cpu), "Unhandled ldtlb");
> +    cpu_abort(env_cpu(env), "Unhandled ldtlb");
>  #else
>      cpu_load_tlb(env);
>  #endif
> @@ -70,7 +67,7 @@ void helper_ldtlb(CPUSH4State *env)
>  static inline void QEMU_NORETURN raise_exception(CPUSH4State *env, int index,
>                                                   uintptr_t retaddr)
>  {
> -    CPUState *cs = CPU(sh_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      cs->exception_index = index;
>      cpu_loop_exit_restore(cs, retaddr);
> @@ -103,7 +100,7 @@ void helper_debug(CPUSH4State *env)
>  
>  void helper_sleep(CPUSH4State *env)
>  {
> -    CPUState *cs = CPU(sh_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      cs->halted = 1;
>      env->in_sleep = 1;
> 

Re: [Qemu-devel] [PATCH for-4.1 v2 23/36] target/sh4: Use env_cpu, env_archcpu

[Philippe Mathieu-Daudé]

On 3/29/19 12:03 AM, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/sh4/cpu.h          |  5 -----
>  linux-user/sh4/cpu_loop.c |  2 +-
>  target/sh4/helper.c       | 26 ++++++++++++--------------
>  target/sh4/op_helper.c    |  9 +++------
>  4 files changed, 16 insertions(+), 26 deletions(-)
> 
> diff --git a/target/sh4/cpu.h b/target/sh4/cpu.h
> index 95094a517a..1f94e7bf7b 100644
> --- a/target/sh4/cpu.h
> +++ b/target/sh4/cpu.h
> @@ -207,11 +207,6 @@ struct SuperHCPU {
>      CPUSH4State env;
>  };
>  
> -static inline SuperHCPU *sh_env_get_cpu(CPUSH4State *env)
> -{
> -    return container_of(env, SuperHCPU, env);
> -}
> -
>  #define ENV_OFFSET offsetof(SuperHCPU, env)
>  
>  void superh_cpu_do_interrupt(CPUState *cpu);
> diff --git a/linux-user/sh4/cpu_loop.c b/linux-user/sh4/cpu_loop.c
> index 47e54b9b61..677c5a461c 100644
> --- a/linux-user/sh4/cpu_loop.c
> +++ b/linux-user/sh4/cpu_loop.c
> @@ -23,7 +23,7 @@
>  
>  void cpu_loop(CPUSH4State *env)
>  {
> -    CPUState *cs = CPU(sh_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>      int trapnr, ret;
>      target_siginfo_t info;
>  
> diff --git a/target/sh4/helper.c b/target/sh4/helper.c
> index 2ff0cf4060..5240da715e 100644
> --- a/target/sh4/helper.c
> +++ b/target/sh4/helper.c
> @@ -238,8 +238,6 @@ static void update_itlb_use(CPUSH4State * env, int itlbnb)
>  
>  static int itlb_replacement(CPUSH4State * env)
>  {
> -    SuperHCPU *cpu = sh_env_get_cpu(env);
> -
>      if ((env->mmucr & 0xe0000000) == 0xe0000000) {
>  	return 0;
>      }
> @@ -252,7 +250,7 @@ static int itlb_replacement(CPUSH4State * env)
>      if ((env->mmucr & 0x2c000000) == 0x00000000) {
>  	return 3;
>      }
> -    cpu_abort(CPU(cpu), "Unhandled itlb_replacement");
> +    cpu_abort(env_cpu(env), "Unhandled itlb_replacement");
>  }
>  
>  /* Find the corresponding entry in the right TLB
> @@ -308,7 +306,7 @@ static int copy_utlb_entry_itlb(CPUSH4State *env, int utlb)
>      itlb = itlb_replacement(env);
>      ientry = &env->itlb[itlb];
>      if (ientry->v) {
> -        tlb_flush_page(CPU(sh_env_get_cpu(env)), ientry->vpn << 10);
> +        tlb_flush_page(env_cpu(env), ientry->vpn << 10);
>      }
>      *ientry = env->utlb[utlb];
>      update_itlb_use(env, itlb);
> @@ -533,14 +531,14 @@ hwaddr superh_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
>  
>  void cpu_load_tlb(CPUSH4State * env)
>  {
> -    SuperHCPU *cpu = sh_env_get_cpu(env);
> +    CPUState *cs = env_cpu(env);
>      int n = cpu_mmucr_urc(env->mmucr);
>      tlb_t * entry = &env->utlb[n];
>  
>      if (entry->v) {
>          /* Overwriting valid entry in utlb. */
>          target_ulong address = entry->vpn << 10;
> -        tlb_flush_page(CPU(cpu), address);
> +        tlb_flush_page(cs, address);
>      }
>  
>      /* Take values into cpu status from registers. */
> @@ -563,7 +561,7 @@ void cpu_load_tlb(CPUSH4State * env)
>          entry->size = 1024 * 1024; /* 1M */
>          break;
>      default:
> -        cpu_abort(CPU(cpu), "Unhandled load_tlb");
> +        cpu_abort(cs, "Unhandled load_tlb");
>          break;
>      }
>      entry->sh   = (uint8_t)cpu_ptel_sh(env->ptel);
> @@ -590,7 +588,7 @@ void cpu_load_tlb(CPUSH4State * env)
>          entry->v = 0;
>      }
>  
> -    tlb_flush(CPU(sh_env_get_cpu(s)));
> +    tlb_flush(env_cpu(s));
>  }
>  
>  uint32_t cpu_sh4_read_mmaped_itlb_addr(CPUSH4State *s,
> @@ -616,7 +614,7 @@ void cpu_sh4_write_mmaped_itlb_addr(CPUSH4State *s, hwaddr addr,
>      if (entry->v) {
>          /* Overwriting valid entry in itlb. */
>          target_ulong address = entry->vpn << 10;
> -        tlb_flush_page(CPU(sh_env_get_cpu(s)), address);
> +        tlb_flush_page(env_cpu(s), address);
>      }
>      entry->asid = asid;
>      entry->vpn = vpn;
> @@ -658,7 +656,7 @@ void cpu_sh4_write_mmaped_itlb_data(CPUSH4State *s, hwaddr addr,
>          if (entry->v) {
>              /* Overwriting valid entry in utlb. */
>              target_ulong address = entry->vpn << 10;
> -            tlb_flush_page(CPU(sh_env_get_cpu(s)), address);
> +            tlb_flush_page(env_cpu(s), address);
>          }
>          entry->ppn = (mem_value & 0x1ffffc00) >> 10;
>          entry->v   = (mem_value & 0x00000100) >> 8;
> @@ -711,7 +709,7 @@ void cpu_sh4_write_mmaped_utlb_addr(CPUSH4State *s, hwaddr addr,
>              if (entry->vpn == vpn
>                  && (!use_asid || entry->asid == asid || entry->sh)) {
>  	        if (utlb_match_entry) {
> -                    CPUState *cs = CPU(sh_env_get_cpu(s));
> +                    CPUState *cs = env_cpu(s);
>  
>  		    /* Multiple TLB Exception */
>                      cs->exception_index = 0x140;
> @@ -743,14 +741,14 @@ void cpu_sh4_write_mmaped_utlb_addr(CPUSH4State *s, hwaddr addr,
>  	}
>  
>          if (needs_tlb_flush) {
> -            tlb_flush_page(CPU(sh_env_get_cpu(s)), vpn << 10);
> +            tlb_flush_page(env_cpu(s), vpn << 10);
>          }
>          
>      } else {
>          int index = (addr & 0x00003f00) >> 8;
>          tlb_t * entry = &s->utlb[index];
>  	if (entry->v) {
> -            CPUState *cs = CPU(sh_env_get_cpu(s));
> +            CPUState *cs = env_cpu(s);
>  
>  	    /* Overwriting valid entry in utlb. */
>              target_ulong address = entry->vpn << 10;
> @@ -805,7 +803,7 @@ void cpu_sh4_write_mmaped_utlb_data(CPUSH4State *s, hwaddr addr,
>          if (entry->v) {
>              /* Overwriting valid entry in utlb. */
>              target_ulong address = entry->vpn << 10;
> -            tlb_flush_page(CPU(sh_env_get_cpu(s)), address);
> +            tlb_flush_page(env_cpu(s), address);
>          }
>          entry->ppn = (mem_value & 0x1ffffc00) >> 10;
>          entry->v   = (mem_value & 0x00000100) >> 8;
> diff --git a/target/sh4/op_helper.c b/target/sh4/op_helper.c
> index 12fba6fc78..11cb68cc1c 100644
> --- a/target/sh4/op_helper.c
> +++ b/target/sh4/op_helper.c
> @@ -58,10 +58,7 @@ void tlb_fill(CPUState *cs, target_ulong addr, int size,
>  void helper_ldtlb(CPUSH4State *env)
>  {
>  #ifdef CONFIG_USER_ONLY
> -    SuperHCPU *cpu = sh_env_get_cpu(env);
> -
> -    /* XXXXX */

Left-over from commit 43dc2a645e0, OK.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> -    cpu_abort(CPU(cpu), "Unhandled ldtlb");
> +    cpu_abort(env_cpu(env), "Unhandled ldtlb");
>  #else
>      cpu_load_tlb(env);
>  #endif
> @@ -70,7 +67,7 @@ void helper_ldtlb(CPUSH4State *env)
>  static inline void QEMU_NORETURN raise_exception(CPUSH4State *env, int index,
>                                                   uintptr_t retaddr)
>  {
> -    CPUState *cs = CPU(sh_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      cs->exception_index = index;
>      cpu_loop_exit_restore(cs, retaddr);
> @@ -103,7 +100,7 @@ void helper_debug(CPUSH4State *env)
>  
>  void helper_sleep(CPUSH4State *env)
>  {
> -    CPUState *cs = CPU(sh_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      cs->halted = 1;
>      env->in_sleep = 1;
> 

Re: [Qemu-devel] [PATCH for-4.1 v2 19/36] target/openrisc: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:44, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 19/36] target/openrisc: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:44, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 20/36] target/ppc: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:43, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 20/36] target/ppc: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:43, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 24/36] target/sparc: Use env_cpu, env_archcpu

[Philippe Mathieu-Daudé]

On 3/29/19 12:03 AM, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  target/sparc/cpu.h          |  5 -----
>  bsd-user/main.c             |  2 +-
>  hw/sparc/leon3.c            |  4 ++--
>  hw/sparc/sun4m.c            |  4 ++--
>  hw/sparc64/sparc64.c        |  2 +-
>  linux-user/sparc/cpu_loop.c |  2 +-
>  target/sparc/fop_helper.c   |  2 +-
>  target/sparc/helper.c       |  8 ++++----
>  target/sparc/ldst_helper.c  | 33 +++++++++++++++------------------
>  target/sparc/mmu_helper.c   | 10 +++++-----
>  10 files changed, 32 insertions(+), 40 deletions(-)
> 
> diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
> index 77dec0d865..bf6f63d029 100644
> --- a/target/sparc/cpu.h
> +++ b/target/sparc/cpu.h
> @@ -532,11 +532,6 @@ struct SPARCCPU {
>      CPUSPARCState env;
>  };
>  
> -static inline SPARCCPU *sparc_env_get_cpu(CPUSPARCState *env)
> -{
> -    return container_of(env, SPARCCPU, env);
> -}
> -
>  #define ENV_OFFSET offsetof(SPARCCPU, env)
>  
>  #ifndef CONFIG_USER_ONLY
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index e554ebdfb3..7a0eb316a2 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -485,7 +485,7 @@ static void flush_windows(CPUSPARCState *env)
>  
>  void cpu_loop(CPUSPARCState *env)
>  {
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>      int trapnr, ret, syscall_nr;
>      //target_siginfo_t info;
>  
> diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
> index 774639af33..ef74bc81c2 100644
> --- a/hw/sparc/leon3.c
> +++ b/hw/sparc/leon3.c
> @@ -91,7 +91,7 @@ static void leon3_set_pil_in(void *opaque, uint32_t pil_in)
>  
>                  env->interrupt_index = TT_EXTINT | i;
>                  if (old_interrupt != env->interrupt_index) {
> -                    cs = CPU(sparc_env_get_cpu(env));
> +                    cs = env_cpu(env);
>                      trace_leon3_set_irq(i);
>                      cpu_interrupt(cs, CPU_INTERRUPT_HARD);
>                  }
> @@ -99,7 +99,7 @@ static void leon3_set_pil_in(void *opaque, uint32_t pil_in)
>              }
>          }
>      } else if (!env->pil_in && (env->interrupt_index & ~15) == TT_EXTINT) {
> -        cs = CPU(sparc_env_get_cpu(env));
> +        cs = env_cpu(env);
>          trace_leon3_reset_irq(env->interrupt_index & 15);
>          env->interrupt_index = 0;
>          cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index ca1e3825d5..a87bef6d4f 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -147,7 +147,7 @@ void cpu_check_irqs(CPUSPARCState *env)
>  
>                  env->interrupt_index = TT_EXTINT | i;
>                  if (old_interrupt != env->interrupt_index) {
> -                    cs = CPU(sparc_env_get_cpu(env));
> +                    cs = env_cpu(env);
>                      trace_sun4m_cpu_interrupt(i);
>                      cpu_interrupt(cs, CPU_INTERRUPT_HARD);
>                  }
> @@ -155,7 +155,7 @@ void cpu_check_irqs(CPUSPARCState *env)
>              }
>          }
>      } else if (!env->pil_in && (env->interrupt_index & ~15) == TT_EXTINT) {
> -        cs = CPU(sparc_env_get_cpu(env));
> +        cs = env_cpu(env);
>          trace_sun4m_cpu_reset_interrupt(env->interrupt_index & 15);
>          env->interrupt_index = 0;
>          cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
> diff --git a/hw/sparc64/sparc64.c b/hw/sparc64/sparc64.c
> index 408388945e..689801f37d 100644
> --- a/hw/sparc64/sparc64.c
> +++ b/hw/sparc64/sparc64.c
> @@ -46,7 +46,7 @@ void cpu_check_irqs(CPUSPARCState *env)
>      if (env->ivec_status & 0x20) {
>          return;
>      }
> -    cs = CPU(sparc_env_get_cpu(env));
> +    cs = env_cpu(env);
>      /* check if TM or SM in SOFTINT are set
>         setting these also causes interrupt 14 */
>      if (env->softint & (SOFTINT_TIMER | SOFTINT_STIMER)) {
> diff --git a/linux-user/sparc/cpu_loop.c b/linux-user/sparc/cpu_loop.c
> index 7d5b337b97..7a4f5792be 100644
> --- a/linux-user/sparc/cpu_loop.c
> +++ b/linux-user/sparc/cpu_loop.c
> @@ -145,7 +145,7 @@ static void flush_windows(CPUSPARCState *env)
>  
>  void cpu_loop (CPUSPARCState *env)
>  {
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>      int trapnr;
>      abi_long ret;
>      target_siginfo_t info;
> diff --git a/target/sparc/fop_helper.c b/target/sparc/fop_helper.c
> index b6642fd1d7..9eb9b75718 100644
> --- a/target/sparc/fop_helper.c
> +++ b/target/sparc/fop_helper.c
> @@ -53,7 +53,7 @@ static target_ulong do_check_ieee_exceptions(CPUSPARCState *env, uintptr_t ra)
>          }
>  
>          if ((fsr & FSR_CEXC_MASK) & ((fsr & FSR_TEM_MASK) >> 23)) {
> -            CPUState *cs = CPU(sparc_env_get_cpu(env));
> +            CPUState *cs = env_cpu(env);
>  
>              /* Unmasked exception, generate a trap.  Note that while
>                 the helper is marked as NO_WG, we can get away with
> diff --git a/target/sparc/helper.c b/target/sparc/helper.c
> index 46232788c8..1a52061fbf 100644
> --- a/target/sparc/helper.c
> +++ b/target/sparc/helper.c
> @@ -26,7 +26,7 @@
>  
>  void cpu_raise_exception_ra(CPUSPARCState *env, int tt, uintptr_t ra)
>  {
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      cs->exception_index = tt;
>      cpu_loop_exit_restore(cs, ra);
> @@ -34,7 +34,7 @@ void cpu_raise_exception_ra(CPUSPARCState *env, int tt, uintptr_t ra)
>  
>  void helper_raise_exception(CPUSPARCState *env, int tt)
>  {
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      cs->exception_index = tt;
>      cpu_loop_exit(cs);
> @@ -42,7 +42,7 @@ void helper_raise_exception(CPUSPARCState *env, int tt)
>  
>  void helper_debug(CPUSPARCState *env)
>  {
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      cs->exception_index = EXCP_DEBUG;
>      cpu_loop_exit(cs);
> @@ -243,7 +243,7 @@ target_ulong helper_tsubcctv(CPUSPARCState *env, target_ulong src1,
>  #ifndef TARGET_SPARC64
>  void helper_power_down(CPUSPARCState *env)
>  {
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      cs->halted = 1;
>      cs->exception_index = EXCP_HLT;
> diff --git a/target/sparc/ldst_helper.c b/target/sparc/ldst_helper.c
> index 5bc090213c..861b420c3e 100644
> --- a/target/sparc/ldst_helper.c
> +++ b/target/sparc/ldst_helper.c
> @@ -122,13 +122,13 @@ static uint64_t ultrasparc_tag_target(uint64_t tag_access_register)
>  
>  static void replace_tlb_entry(SparcTLBEntry *tlb,
>                                uint64_t tlb_tag, uint64_t tlb_tte,
> -                              CPUSPARCState *env1)
> +                              CPUSPARCState *env)
>  {
>      target_ulong mask, size, va, offset;
>  
>      /* flush page range if translation is valid */
>      if (TTE_IS_VALID(tlb->tte)) {
> -        CPUState *cs = CPU(sparc_env_get_cpu(env1));
> +        CPUState *cs = env_cpu(env);
>  
>          size = 8192ULL << 3 * TTE_PGSIZE(tlb->tte);
>          mask = 1ULL + ~size;
> @@ -499,7 +499,7 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong addr,
>  {
>      int size = 1 << (memop & MO_SIZE);
>      int sign = memop & MO_SIGN;
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>      uint64_t ret = 0;
>  #if defined(DEBUG_MXCC) || defined(DEBUG_ASI)
>      uint32_t last_addr = addr;
> @@ -725,8 +725,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, uint64_t val,
>                     int asi, uint32_t memop)
>  {
>      int size = 1 << (memop & MO_SIZE);
> -    SPARCCPU *cpu = sparc_env_get_cpu(env);
> -    CPUState *cs = CPU(cpu);
> +    CPUState *cs = env_cpu(env);
>  
>      do_check_align(env, addr, size - 1, GETPC());
>      switch (asi) {
> @@ -874,13 +873,13 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, uint64_t val,
>              DPRINTF_MMU("mmu flush level %d\n", mmulev);
>              switch (mmulev) {
>              case 0: /* flush page */
> -                tlb_flush_page(CPU(cpu), addr & 0xfffff000);
> +                tlb_flush_page(cs, addr & 0xfffff000);
>                  break;
>              case 1: /* flush segment (256k) */
>              case 2: /* flush region (16M) */
>              case 3: /* flush context (4G) */
>              case 4: /* flush entire */
> -                tlb_flush(CPU(cpu));
> +                tlb_flush(cs);
>                  break;
>              default:
>                  break;
> @@ -905,7 +904,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, uint64_t val,
>                     are invalid in normal mode.  */
>                  if ((oldreg ^ env->mmuregs[reg])
>                      & (MMU_NF | env->def.mmu_bm)) {
> -                    tlb_flush(CPU(cpu));
> +                    tlb_flush(cs);
>                  }
>                  break;
>              case 1: /* Context Table Pointer Register */
> @@ -916,7 +915,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, uint64_t val,
>                  if (oldreg != env->mmuregs[reg]) {
>                      /* we flush when the MMU context changes because
>                         QEMU has no MMU context support */
> -                    tlb_flush(CPU(cpu));
> +                    tlb_flush(cs);
>                  }
>                  break;
>              case 3: /* Synchronous Fault Status Register with Clear */
> @@ -1027,8 +1026,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, uint64_t val,
>      case ASI_USERTXT: /* User code access, XXX */
>      case ASI_KERNELTXT: /* Supervisor code access, XXX */
>      default:
> -        cpu_unassigned_access(CPU(sparc_env_get_cpu(env)),
> -                              addr, true, false, asi, size);
> +        cpu_unassigned_access(cs, addr, true, false, asi, size);
>          break;
>  
>      case ASI_USERDATA: /* User data access */
> @@ -1175,7 +1173,7 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong addr,
>  {
>      int size = 1 << (memop & MO_SIZE);
>      int sign = memop & MO_SIGN;
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>      uint64_t ret = 0;
>  #if defined(DEBUG_ASI)
>      target_ulong last_addr = addr;
> @@ -1481,8 +1479,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, target_ulong val,
>                     int asi, uint32_t memop)
>  {
>      int size = 1 << (memop & MO_SIZE);
> -    SPARCCPU *cpu = sparc_env_get_cpu(env);
> -    CPUState *cs = CPU(cpu);
> +    CPUState *cs = env_cpu(env);
>  
>  #ifdef DEBUG_ASI
>      dump_asi("write", addr, asi, size, val);
> @@ -1686,13 +1683,13 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, target_ulong val,
>                  env->dmmu.mmu_primary_context = val;
>                  /* can be optimized to only flush MMU_USER_IDX
>                     and MMU_KERNEL_IDX entries */
> -                tlb_flush(CPU(cpu));
> +                tlb_flush(cs);
>                  break;
>              case 2: /* Secondary context */
>                  env->dmmu.mmu_secondary_context = val;
>                  /* can be optimized to only flush MMU_USER_SECONDARY_IDX
>                     and MMU_KERNEL_SECONDARY_IDX entries */
> -                tlb_flush(CPU(cpu));
> +                tlb_flush(cs);
>                  break;
>              case 5: /* TSB access */
>                  DPRINTF_MMU("dmmu TSB write: 0x%016" PRIx64 " -> 0x%016"
> @@ -1768,13 +1765,13 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, target_ulong val,
>            case 1:
>                env->dmmu.mmu_primary_context = val;
>                env->immu.mmu_primary_context = val;
> -              tlb_flush_by_mmuidx(CPU(cpu),
> +              tlb_flush_by_mmuidx(cs,
>                                    (1 << MMU_USER_IDX) | (1 << MMU_KERNEL_IDX));
>                break;
>            case 2:
>                env->dmmu.mmu_secondary_context = val;
>                env->immu.mmu_secondary_context = val;
> -              tlb_flush_by_mmuidx(CPU(cpu),
> +              tlb_flush_by_mmuidx(cs,
>                                    (1 << MMU_USER_SECONDARY_IDX) |
>                                    (1 << MMU_KERNEL_SECONDARY_IDX));
>                break;
> diff --git a/target/sparc/mmu_helper.c b/target/sparc/mmu_helper.c
> index 135a9c9d9b..0366c26246 100644
> --- a/target/sparc/mmu_helper.c
> +++ b/target/sparc/mmu_helper.c
> @@ -95,7 +95,7 @@ static int get_physical_address(CPUSPARCState *env, hwaddr *physical,
>      uint32_t pde;
>      int error_code = 0, is_dirty, is_user;
>      unsigned long page_offset;
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      is_user = mmu_idx == MMU_USER_IDX;
>  
> @@ -255,7 +255,7 @@ int sparc_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int size, int rw,
>  
>  target_ulong mmu_probe(CPUSPARCState *env, target_ulong address, int mmulev)
>  {
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>      hwaddr pde_ptr;
>      uint32_t pde;
>  
> @@ -322,7 +322,7 @@ target_ulong mmu_probe(CPUSPARCState *env, target_ulong address, int mmulev)
>  
>  void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUSPARCState *env)
>  {
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>      target_ulong va, va1, va2;
>      unsigned int n, m, o;
>      hwaddr pde_ptr, pa;
> @@ -481,7 +481,7 @@ static int get_physical_address_data(CPUSPARCState *env,
>                                       hwaddr *physical, int *prot,
>                                       target_ulong address, int rw, int mmu_idx)
>  {
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>      unsigned int i;
>      uint64_t context;
>      uint64_t sfsr = 0;
> @@ -599,7 +599,7 @@ static int get_physical_address_code(CPUSPARCState *env,
>                                       hwaddr *physical, int *prot,
>                                       target_ulong address, int mmu_idx)
>  {
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>      unsigned int i;
>      uint64_t context;
>      bool is_user = false;
> 

Re: [Qemu-devel] [PATCH for-4.1 v2 24/36] target/sparc: Use env_cpu, env_archcpu

[Philippe Mathieu-Daudé]

On 3/29/19 12:03 AM, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  target/sparc/cpu.h          |  5 -----
>  bsd-user/main.c             |  2 +-
>  hw/sparc/leon3.c            |  4 ++--
>  hw/sparc/sun4m.c            |  4 ++--
>  hw/sparc64/sparc64.c        |  2 +-
>  linux-user/sparc/cpu_loop.c |  2 +-
>  target/sparc/fop_helper.c   |  2 +-
>  target/sparc/helper.c       |  8 ++++----
>  target/sparc/ldst_helper.c  | 33 +++++++++++++++------------------
>  target/sparc/mmu_helper.c   | 10 +++++-----
>  10 files changed, 32 insertions(+), 40 deletions(-)
> 
> diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
> index 77dec0d865..bf6f63d029 100644
> --- a/target/sparc/cpu.h
> +++ b/target/sparc/cpu.h
> @@ -532,11 +532,6 @@ struct SPARCCPU {
>      CPUSPARCState env;
>  };
>  
> -static inline SPARCCPU *sparc_env_get_cpu(CPUSPARCState *env)
> -{
> -    return container_of(env, SPARCCPU, env);
> -}
> -
>  #define ENV_OFFSET offsetof(SPARCCPU, env)
>  
>  #ifndef CONFIG_USER_ONLY
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index e554ebdfb3..7a0eb316a2 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -485,7 +485,7 @@ static void flush_windows(CPUSPARCState *env)
>  
>  void cpu_loop(CPUSPARCState *env)
>  {
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>      int trapnr, ret, syscall_nr;
>      //target_siginfo_t info;
>  
> diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
> index 774639af33..ef74bc81c2 100644
> --- a/hw/sparc/leon3.c
> +++ b/hw/sparc/leon3.c
> @@ -91,7 +91,7 @@ static void leon3_set_pil_in(void *opaque, uint32_t pil_in)
>  
>                  env->interrupt_index = TT_EXTINT | i;
>                  if (old_interrupt != env->interrupt_index) {
> -                    cs = CPU(sparc_env_get_cpu(env));
> +                    cs = env_cpu(env);
>                      trace_leon3_set_irq(i);
>                      cpu_interrupt(cs, CPU_INTERRUPT_HARD);
>                  }
> @@ -99,7 +99,7 @@ static void leon3_set_pil_in(void *opaque, uint32_t pil_in)
>              }
>          }
>      } else if (!env->pil_in && (env->interrupt_index & ~15) == TT_EXTINT) {
> -        cs = CPU(sparc_env_get_cpu(env));
> +        cs = env_cpu(env);
>          trace_leon3_reset_irq(env->interrupt_index & 15);
>          env->interrupt_index = 0;
>          cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index ca1e3825d5..a87bef6d4f 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -147,7 +147,7 @@ void cpu_check_irqs(CPUSPARCState *env)
>  
>                  env->interrupt_index = TT_EXTINT | i;
>                  if (old_interrupt != env->interrupt_index) {
> -                    cs = CPU(sparc_env_get_cpu(env));
> +                    cs = env_cpu(env);
>                      trace_sun4m_cpu_interrupt(i);
>                      cpu_interrupt(cs, CPU_INTERRUPT_HARD);
>                  }
> @@ -155,7 +155,7 @@ void cpu_check_irqs(CPUSPARCState *env)
>              }
>          }
>      } else if (!env->pil_in && (env->interrupt_index & ~15) == TT_EXTINT) {
> -        cs = CPU(sparc_env_get_cpu(env));
> +        cs = env_cpu(env);
>          trace_sun4m_cpu_reset_interrupt(env->interrupt_index & 15);
>          env->interrupt_index = 0;
>          cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
> diff --git a/hw/sparc64/sparc64.c b/hw/sparc64/sparc64.c
> index 408388945e..689801f37d 100644
> --- a/hw/sparc64/sparc64.c
> +++ b/hw/sparc64/sparc64.c
> @@ -46,7 +46,7 @@ void cpu_check_irqs(CPUSPARCState *env)
>      if (env->ivec_status & 0x20) {
>          return;
>      }
> -    cs = CPU(sparc_env_get_cpu(env));
> +    cs = env_cpu(env);
>      /* check if TM or SM in SOFTINT are set
>         setting these also causes interrupt 14 */
>      if (env->softint & (SOFTINT_TIMER | SOFTINT_STIMER)) {
> diff --git a/linux-user/sparc/cpu_loop.c b/linux-user/sparc/cpu_loop.c
> index 7d5b337b97..7a4f5792be 100644
> --- a/linux-user/sparc/cpu_loop.c
> +++ b/linux-user/sparc/cpu_loop.c
> @@ -145,7 +145,7 @@ static void flush_windows(CPUSPARCState *env)
>  
>  void cpu_loop (CPUSPARCState *env)
>  {
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>      int trapnr;
>      abi_long ret;
>      target_siginfo_t info;
> diff --git a/target/sparc/fop_helper.c b/target/sparc/fop_helper.c
> index b6642fd1d7..9eb9b75718 100644
> --- a/target/sparc/fop_helper.c
> +++ b/target/sparc/fop_helper.c
> @@ -53,7 +53,7 @@ static target_ulong do_check_ieee_exceptions(CPUSPARCState *env, uintptr_t ra)
>          }
>  
>          if ((fsr & FSR_CEXC_MASK) & ((fsr & FSR_TEM_MASK) >> 23)) {
> -            CPUState *cs = CPU(sparc_env_get_cpu(env));
> +            CPUState *cs = env_cpu(env);
>  
>              /* Unmasked exception, generate a trap.  Note that while
>                 the helper is marked as NO_WG, we can get away with
> diff --git a/target/sparc/helper.c b/target/sparc/helper.c
> index 46232788c8..1a52061fbf 100644
> --- a/target/sparc/helper.c
> +++ b/target/sparc/helper.c
> @@ -26,7 +26,7 @@
>  
>  void cpu_raise_exception_ra(CPUSPARCState *env, int tt, uintptr_t ra)
>  {
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      cs->exception_index = tt;
>      cpu_loop_exit_restore(cs, ra);
> @@ -34,7 +34,7 @@ void cpu_raise_exception_ra(CPUSPARCState *env, int tt, uintptr_t ra)
>  
>  void helper_raise_exception(CPUSPARCState *env, int tt)
>  {
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      cs->exception_index = tt;
>      cpu_loop_exit(cs);
> @@ -42,7 +42,7 @@ void helper_raise_exception(CPUSPARCState *env, int tt)
>  
>  void helper_debug(CPUSPARCState *env)
>  {
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      cs->exception_index = EXCP_DEBUG;
>      cpu_loop_exit(cs);
> @@ -243,7 +243,7 @@ target_ulong helper_tsubcctv(CPUSPARCState *env, target_ulong src1,
>  #ifndef TARGET_SPARC64
>  void helper_power_down(CPUSPARCState *env)
>  {
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      cs->halted = 1;
>      cs->exception_index = EXCP_HLT;
> diff --git a/target/sparc/ldst_helper.c b/target/sparc/ldst_helper.c
> index 5bc090213c..861b420c3e 100644
> --- a/target/sparc/ldst_helper.c
> +++ b/target/sparc/ldst_helper.c
> @@ -122,13 +122,13 @@ static uint64_t ultrasparc_tag_target(uint64_t tag_access_register)
>  
>  static void replace_tlb_entry(SparcTLBEntry *tlb,
>                                uint64_t tlb_tag, uint64_t tlb_tte,
> -                              CPUSPARCState *env1)
> +                              CPUSPARCState *env)
>  {
>      target_ulong mask, size, va, offset;
>  
>      /* flush page range if translation is valid */
>      if (TTE_IS_VALID(tlb->tte)) {
> -        CPUState *cs = CPU(sparc_env_get_cpu(env1));
> +        CPUState *cs = env_cpu(env);
>  
>          size = 8192ULL << 3 * TTE_PGSIZE(tlb->tte);
>          mask = 1ULL + ~size;
> @@ -499,7 +499,7 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong addr,
>  {
>      int size = 1 << (memop & MO_SIZE);
>      int sign = memop & MO_SIGN;
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>      uint64_t ret = 0;
>  #if defined(DEBUG_MXCC) || defined(DEBUG_ASI)
>      uint32_t last_addr = addr;
> @@ -725,8 +725,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, uint64_t val,
>                     int asi, uint32_t memop)
>  {
>      int size = 1 << (memop & MO_SIZE);
> -    SPARCCPU *cpu = sparc_env_get_cpu(env);
> -    CPUState *cs = CPU(cpu);
> +    CPUState *cs = env_cpu(env);
>  
>      do_check_align(env, addr, size - 1, GETPC());
>      switch (asi) {
> @@ -874,13 +873,13 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, uint64_t val,
>              DPRINTF_MMU("mmu flush level %d\n", mmulev);
>              switch (mmulev) {
>              case 0: /* flush page */
> -                tlb_flush_page(CPU(cpu), addr & 0xfffff000);
> +                tlb_flush_page(cs, addr & 0xfffff000);
>                  break;
>              case 1: /* flush segment (256k) */
>              case 2: /* flush region (16M) */
>              case 3: /* flush context (4G) */
>              case 4: /* flush entire */
> -                tlb_flush(CPU(cpu));
> +                tlb_flush(cs);
>                  break;
>              default:
>                  break;
> @@ -905,7 +904,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, uint64_t val,
>                     are invalid in normal mode.  */
>                  if ((oldreg ^ env->mmuregs[reg])
>                      & (MMU_NF | env->def.mmu_bm)) {
> -                    tlb_flush(CPU(cpu));
> +                    tlb_flush(cs);
>                  }
>                  break;
>              case 1: /* Context Table Pointer Register */
> @@ -916,7 +915,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, uint64_t val,
>                  if (oldreg != env->mmuregs[reg]) {
>                      /* we flush when the MMU context changes because
>                         QEMU has no MMU context support */
> -                    tlb_flush(CPU(cpu));
> +                    tlb_flush(cs);
>                  }
>                  break;
>              case 3: /* Synchronous Fault Status Register with Clear */
> @@ -1027,8 +1026,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, uint64_t val,
>      case ASI_USERTXT: /* User code access, XXX */
>      case ASI_KERNELTXT: /* Supervisor code access, XXX */
>      default:
> -        cpu_unassigned_access(CPU(sparc_env_get_cpu(env)),
> -                              addr, true, false, asi, size);
> +        cpu_unassigned_access(cs, addr, true, false, asi, size);
>          break;
>  
>      case ASI_USERDATA: /* User data access */
> @@ -1175,7 +1173,7 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong addr,
>  {
>      int size = 1 << (memop & MO_SIZE);
>      int sign = memop & MO_SIGN;
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>      uint64_t ret = 0;
>  #if defined(DEBUG_ASI)
>      target_ulong last_addr = addr;
> @@ -1481,8 +1479,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, target_ulong val,
>                     int asi, uint32_t memop)
>  {
>      int size = 1 << (memop & MO_SIZE);
> -    SPARCCPU *cpu = sparc_env_get_cpu(env);
> -    CPUState *cs = CPU(cpu);
> +    CPUState *cs = env_cpu(env);
>  
>  #ifdef DEBUG_ASI
>      dump_asi("write", addr, asi, size, val);
> @@ -1686,13 +1683,13 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, target_ulong val,
>                  env->dmmu.mmu_primary_context = val;
>                  /* can be optimized to only flush MMU_USER_IDX
>                     and MMU_KERNEL_IDX entries */
> -                tlb_flush(CPU(cpu));
> +                tlb_flush(cs);
>                  break;
>              case 2: /* Secondary context */
>                  env->dmmu.mmu_secondary_context = val;
>                  /* can be optimized to only flush MMU_USER_SECONDARY_IDX
>                     and MMU_KERNEL_SECONDARY_IDX entries */
> -                tlb_flush(CPU(cpu));
> +                tlb_flush(cs);
>                  break;
>              case 5: /* TSB access */
>                  DPRINTF_MMU("dmmu TSB write: 0x%016" PRIx64 " -> 0x%016"
> @@ -1768,13 +1765,13 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, target_ulong val,
>            case 1:
>                env->dmmu.mmu_primary_context = val;
>                env->immu.mmu_primary_context = val;
> -              tlb_flush_by_mmuidx(CPU(cpu),
> +              tlb_flush_by_mmuidx(cs,
>                                    (1 << MMU_USER_IDX) | (1 << MMU_KERNEL_IDX));
>                break;
>            case 2:
>                env->dmmu.mmu_secondary_context = val;
>                env->immu.mmu_secondary_context = val;
> -              tlb_flush_by_mmuidx(CPU(cpu),
> +              tlb_flush_by_mmuidx(cs,
>                                    (1 << MMU_USER_SECONDARY_IDX) |
>                                    (1 << MMU_KERNEL_SECONDARY_IDX));
>                break;
> diff --git a/target/sparc/mmu_helper.c b/target/sparc/mmu_helper.c
> index 135a9c9d9b..0366c26246 100644
> --- a/target/sparc/mmu_helper.c
> +++ b/target/sparc/mmu_helper.c
> @@ -95,7 +95,7 @@ static int get_physical_address(CPUSPARCState *env, hwaddr *physical,
>      uint32_t pde;
>      int error_code = 0, is_dirty, is_user;
>      unsigned long page_offset;
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>  
>      is_user = mmu_idx == MMU_USER_IDX;
>  
> @@ -255,7 +255,7 @@ int sparc_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int size, int rw,
>  
>  target_ulong mmu_probe(CPUSPARCState *env, target_ulong address, int mmulev)
>  {
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>      hwaddr pde_ptr;
>      uint32_t pde;
>  
> @@ -322,7 +322,7 @@ target_ulong mmu_probe(CPUSPARCState *env, target_ulong address, int mmulev)
>  
>  void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUSPARCState *env)
>  {
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>      target_ulong va, va1, va2;
>      unsigned int n, m, o;
>      hwaddr pde_ptr, pa;
> @@ -481,7 +481,7 @@ static int get_physical_address_data(CPUSPARCState *env,
>                                       hwaddr *physical, int *prot,
>                                       target_ulong address, int rw, int mmu_idx)
>  {
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>      unsigned int i;
>      uint64_t context;
>      uint64_t sfsr = 0;
> @@ -599,7 +599,7 @@ static int get_physical_address_code(CPUSPARCState *env,
>                                       hwaddr *physical, int *prot,
>                                       target_ulong address, int mmu_idx)
>  {
> -    CPUState *cs = CPU(sparc_env_get_cpu(env));
> +    CPUState *cs = env_cpu(env);
>      unsigned int i;
>      uint64_t context;
>      bool is_user = false;
> 

Re: [Qemu-devel] [PATCH for-4.1 v2 22/36] target/s390x: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:35, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/s390x/cpu.h          |  5 ----
>  linux-user/s390x/cpu_loop.c |  2 +-
>  target/s390x/cc_helper.c    |  5 ++--
>  target/s390x/diag.c         |  2 +-
>  target/s390x/excp_helper.c  |  6 ++---
>  target/s390x/fpu_helper.c   |  4 +--
>  target/s390x/helper.c       |  7 +++---
>  target/s390x/int_helper.c   |  3 +--
>  target/s390x/interrupt.c    |  6 ++---
>  target/s390x/mem_helper.c   | 28 ++++++++-------------
>  target/s390x/misc_helper.c  | 50 ++++++++++++++++++-------------------
>  target/s390x/mmu_helper.c   |  8 +++---
>  target/s390x/sigp.c         |  4 +--
>  13 files changed, 56 insertions(+), 74 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 22/36] target/s390x: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:35, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/s390x/cpu.h          |  5 ----
>  linux-user/s390x/cpu_loop.c |  2 +-
>  target/s390x/cc_helper.c    |  5 ++--
>  target/s390x/diag.c         |  2 +-
>  target/s390x/excp_helper.c  |  6 ++---
>  target/s390x/fpu_helper.c   |  4 +--
>  target/s390x/helper.c       |  7 +++---
>  target/s390x/int_helper.c   |  3 +--
>  target/s390x/interrupt.c    |  6 ++---
>  target/s390x/mem_helper.c   | 28 ++++++++-------------
>  target/s390x/misc_helper.c  | 50 ++++++++++++++++++-------------------
>  target/s390x/mmu_helper.c   |  8 +++---
>  target/s390x/sigp.c         |  4 +--
>  13 files changed, 56 insertions(+), 74 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 25/36] target/tilegx: Use env_cpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:34, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/tilegx/cpu.h          | 5 -----
>  linux-user/tilegx/cpu_loop.c | 2 +-
>  target/tilegx/helper.c       | 2 +-
>  3 files changed, 2 insertions(+), 7 deletions(-)
>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 25/36] target/tilegx: Use env_cpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:34, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/tilegx/cpu.h          | 5 -----
>  linux-user/tilegx/cpu_loop.c | 2 +-
>  target/tilegx/helper.c       | 2 +-
>  3 files changed, 2 insertions(+), 7 deletions(-)
>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 26/36] target/tricore: Use env_cpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:31, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/tricore/cpu.h       | 5 -----
>  target/tricore/op_helper.c | 4 ++--
>  2 files changed, 2 insertions(+), 7 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 26/36] target/tricore: Use env_cpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:31, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/tricore/cpu.h       | 5 -----
>  target/tricore/op_helper.c | 4 ++--
>  2 files changed, 2 insertions(+), 7 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 27/36] target/unicore32: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:26, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 27/36] target/unicore32: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:26, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 28/36] target/xtensa: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:38, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> @@ -712,10 +707,15 @@ static inline int cpu_mmu_index(CPUXtensaState *env, bool ifetch)
>  #define XTENSA_CSBASE_LBEG_OFF_MASK 0x00ff0000
>  #define XTENSA_CSBASE_LBEG_OFF_SHIFT 16
>
> +typedef CPUXtensaState CPUArchState;
> +typedef XtensaCPU ArchCPU;
> +
> +#include "exec/cpu-all.h"

Shouldn't this bit be in an earlier patch ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 28/36] target/xtensa: Use env_cpu, env_archcpu

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:38, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> @@ -712,10 +707,15 @@ static inline int cpu_mmu_index(CPUXtensaState *env, bool ifetch)
>  #define XTENSA_CSBASE_LBEG_OFF_MASK 0x00ff0000
>  #define XTENSA_CSBASE_LBEG_OFF_SHIFT 16
>
> +typedef CPUXtensaState CPUArchState;
> +typedef XtensaCPU ArchCPU;
> +
> +#include "exec/cpu-all.h"

Shouldn't this bit be in an earlier patch ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 28/36] target/xtensa: Use env_cpu, env_archcpu

[Richard Henderson]

On 4/29/19 7:36 AM, Peter Maydell wrote:
> On Thu, 28 Mar 2019 at 23:38, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>> @@ -712,10 +707,15 @@ static inline int cpu_mmu_index(CPUXtensaState *env, bool ifetch)
>>  #define XTENSA_CSBASE_LBEG_OFF_MASK 0x00ff0000
>>  #define XTENSA_CSBASE_LBEG_OFF_SHIFT 16
>>
>> +typedef CPUXtensaState CPUArchState;
>> +typedef XtensaCPU ArchCPU;
>> +
>> +#include "exec/cpu-all.h"
> 
> Shouldn't this bit be in an earlier patch ?

The diff is just funny here.

As with cris, it's really moving the following function below
exec/cpu-all.h, so that env_archcpu is defined.


r~

Re: [Qemu-devel] [PATCH for-4.1 v2 28/36] target/xtensa: Use env_cpu, env_archcpu

[Richard Henderson]

On 4/29/19 7:36 AM, Peter Maydell wrote:
> On Thu, 28 Mar 2019 at 23:38, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>> @@ -712,10 +707,15 @@ static inline int cpu_mmu_index(CPUXtensaState *env, bool ifetch)
>>  #define XTENSA_CSBASE_LBEG_OFF_MASK 0x00ff0000
>>  #define XTENSA_CSBASE_LBEG_OFF_SHIFT 16
>>
>> +typedef CPUXtensaState CPUArchState;
>> +typedef XtensaCPU ArchCPU;
>> +
>> +#include "exec/cpu-all.h"
> 
> Shouldn't this bit be in an earlier patch ?

The diff is just funny here.

As with cris, it's really moving the following function below
exec/cpu-all.h, so that env_archcpu is defined.


r~

Re: [Qemu-devel] [PATCH for-4.1 v2 28/36] target/xtensa: Use env_cpu, env_archcpu

[Peter Maydell]

On Mon, 29 Apr 2019 at 16:20, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 4/29/19 7:36 AM, Peter Maydell wrote:
> > On Thu, 28 Mar 2019 at 23:38, Richard Henderson
> > <richard.henderson@linaro.org> wrote:
> >>
> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> >> ---
> >> @@ -712,10 +707,15 @@ static inline int cpu_mmu_index(CPUXtensaState *env, bool ifetch)
> >>  #define XTENSA_CSBASE_LBEG_OFF_MASK 0x00ff0000
> >>  #define XTENSA_CSBASE_LBEG_OFF_SHIFT 16
> >>
> >> +typedef CPUXtensaState CPUArchState;
> >> +typedef XtensaCPU ArchCPU;
> >> +
> >> +#include "exec/cpu-all.h"
> >
> > Shouldn't this bit be in an earlier patch ?
>
> The diff is just funny here.
>
> As with cris, it's really moving the following function below
> exec/cpu-all.h, so that env_archcpu is defined.

Oh, I see. With a note in the commit message about the
function definition moving

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 28/36] target/xtensa: Use env_cpu, env_archcpu

[Peter Maydell]

On Mon, 29 Apr 2019 at 16:20, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 4/29/19 7:36 AM, Peter Maydell wrote:
> > On Thu, 28 Mar 2019 at 23:38, Richard Henderson
> > <richard.henderson@linaro.org> wrote:
> >>
> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> >> ---
> >> @@ -712,10 +707,15 @@ static inline int cpu_mmu_index(CPUXtensaState *env, bool ifetch)
> >>  #define XTENSA_CSBASE_LBEG_OFF_MASK 0x00ff0000
> >>  #define XTENSA_CSBASE_LBEG_OFF_SHIFT 16
> >>
> >> +typedef CPUXtensaState CPUArchState;
> >> +typedef XtensaCPU ArchCPU;
> >> +
> >> +#include "exec/cpu-all.h"
> >
> > Shouldn't this bit be in an earlier patch ?
>
> The diff is just funny here.
>
> As with cris, it's really moving the following function below
> exec/cpu-all.h, so that env_archcpu is defined.

Oh, I see. With a note in the commit message about the
function definition moving

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 29/36] cpu: Move ENV_OFFSET to exec/gen-icount.h

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:32, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Now that we have ArchCPU, we can define this generically,
> in the one place that needs it.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 29/36] cpu: Move ENV_OFFSET to exec/gen-icount.h

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:32, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Now that we have ArchCPU, we can define this generically,
> in the one place that needs it.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 30/36] cpu: Introduce cpu_set_cpustate_pointers

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:23, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Consolidate some boilerplate from foo_cpu_initfn.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -371,6 +371,17 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
>
>  int cpu_exec(CPUState *cpu);
>
> +/**
> + * cpu_set_cpustate_pointers(cpu)
> + * @cpu: The cpu object
> + *
> + * Set the generic pointers in CPUState into the outer object.
> + */
> +static inline void cpu_set_cpustate_pointers(ArchCPU *cpu)
> +{
> +    cpu->parent_obj.env_ptr = &cpu->env;
> +}
> +
>  /**
>   * env_archcpu(env)
>   * @env: The architecture environment
> @@ -392,5 +403,4 @@ static inline CPUState *env_cpu(CPUArchState *env)
>  {
>      return &env_archcpu(env)->parent_obj;
>  }
> -

Stray blank line deletion.

>  #endif /* CPU_ALL_H */

> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index 698dd9cb82..790670ebeb 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -282,17 +282,18 @@ static void s390_cpu_initfn(Object *obj)
>  {
>      CPUState *cs = CPU(obj);
>      S390CPU *cpu = S390_CPU(obj);
> -    CPUS390XState *env = &cpu->env;
>
> -    cs->env_ptr = env;
> +    cpu_set_cpustate_pointers(cpu);
>      cs->halted = 1;
>      cs->exception_index = EXCP_HLT;
>      object_property_add(obj, "crash-information", "GuestPanicInformation",
>                          s390_cpu_get_crash_info_qom, NULL, NULL, NULL, NULL);
>      s390_cpu_model_register_props(obj);
>  #if !defined(CONFIG_USER_ONLY)
> -    env->tod_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_tod_timer, cpu);
> -    env->cpu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_cpu_timer, cpu);
> +    cpu->env.tod_timer =
> +        timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_tod_timer, cpu);
> +    cpu->env.cpu_timer =
> +        timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_cpu_timer, cpu);
>      s390_cpu_set_state(S390_CPU_STATE_STOPPED, cpu);
>  #endif

I would have left the local variable so that we didn't
need to change these lines, but whatever.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 30/36] cpu: Introduce cpu_set_cpustate_pointers

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:23, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Consolidate some boilerplate from foo_cpu_initfn.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -371,6 +371,17 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
>
>  int cpu_exec(CPUState *cpu);
>
> +/**
> + * cpu_set_cpustate_pointers(cpu)
> + * @cpu: The cpu object
> + *
> + * Set the generic pointers in CPUState into the outer object.
> + */
> +static inline void cpu_set_cpustate_pointers(ArchCPU *cpu)
> +{
> +    cpu->parent_obj.env_ptr = &cpu->env;
> +}
> +
>  /**
>   * env_archcpu(env)
>   * @env: The architecture environment
> @@ -392,5 +403,4 @@ static inline CPUState *env_cpu(CPUArchState *env)
>  {
>      return &env_archcpu(env)->parent_obj;
>  }
> -

Stray blank line deletion.

>  #endif /* CPU_ALL_H */

> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index 698dd9cb82..790670ebeb 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -282,17 +282,18 @@ static void s390_cpu_initfn(Object *obj)
>  {
>      CPUState *cs = CPU(obj);
>      S390CPU *cpu = S390_CPU(obj);
> -    CPUS390XState *env = &cpu->env;
>
> -    cs->env_ptr = env;
> +    cpu_set_cpustate_pointers(cpu);
>      cs->halted = 1;
>      cs->exception_index = EXCP_HLT;
>      object_property_add(obj, "crash-information", "GuestPanicInformation",
>                          s390_cpu_get_crash_info_qom, NULL, NULL, NULL, NULL);
>      s390_cpu_model_register_props(obj);
>  #if !defined(CONFIG_USER_ONLY)
> -    env->tod_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_tod_timer, cpu);
> -    env->cpu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_cpu_timer, cpu);
> +    cpu->env.tod_timer =
> +        timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_tod_timer, cpu);
> +    cpu->env.cpu_timer =
> +        timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_cpu_timer, cpu);
>      s390_cpu_set_state(S390_CPU_STATE_STOPPED, cpu);
>  #endif

I would have left the local variable so that we didn't
need to change these lines, but whatever.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 30/36] cpu: Introduce cpu_set_cpustate_pointers

[Richard Henderson]

On 4/29/19 7:40 AM, Peter Maydell wrote:
>> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
>> index 698dd9cb82..790670ebeb 100644
>> --- a/target/s390x/cpu.c
>> +++ b/target/s390x/cpu.c
>> @@ -282,17 +282,18 @@ static void s390_cpu_initfn(Object *obj)
>>  {
>>      CPUState *cs = CPU(obj);
>>      S390CPU *cpu = S390_CPU(obj);
>> -    CPUS390XState *env = &cpu->env;
>>
>> -    cs->env_ptr = env;
>> +    cpu_set_cpustate_pointers(cpu);
>>      cs->halted = 1;
>>      cs->exception_index = EXCP_HLT;
>>      object_property_add(obj, "crash-information", "GuestPanicInformation",
>>                          s390_cpu_get_crash_info_qom, NULL, NULL, NULL, NULL);
>>      s390_cpu_model_register_props(obj);
>>  #if !defined(CONFIG_USER_ONLY)
>> -    env->tod_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_tod_timer, cpu);
>> -    env->cpu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_cpu_timer, cpu);
>> +    cpu->env.tod_timer =
>> +        timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_tod_timer, cpu);
>> +    cpu->env.cpu_timer =
>> +        timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_cpu_timer, cpu);
>>      s390_cpu_set_state(S390_CPU_STATE_STOPPED, cpu);
>>  #endif
> 
> I would have left the local variable so that we didn't
> need to change these lines, but whatever.

Then the variable is unused for CONFIG_USER_ONLY.
It's easier to suppress the warning by propagating.


r~

Re: [Qemu-devel] [PATCH for-4.1 v2 31/36] cpu: Introduce CPUNegativeOffsetState

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:35, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Nothing in there so far, but all of the plumbing done
> within the target ArchCPU state.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> +/*
> + * This structure must be placed in ArchCPU immedately
> + * before CPUArchState, as a field named "neg".
> + */
> +typedef struct CPUNegativeOffsetState {
> +    /* Empty */
> +} CPUNegativeOffsetState;

It would be nice if we could do a compile-time assert that
 offsetof(CPUArchState, neg) + sizeof(CPUNegativeOffsetState)
  == offsetof(CPUArchState, env)

but I guess the possibility of within-struct padding
between the two structs makes that dubious. Or does our
generated code rely on there being no padding between neg
and env anyway ?

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 31/36] cpu: Introduce CPUNegativeOffsetState

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:35, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Nothing in there so far, but all of the plumbing done
> within the target ArchCPU state.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> +/*
> + * This structure must be placed in ArchCPU immedately
> + * before CPUArchState, as a field named "neg".
> + */
> +typedef struct CPUNegativeOffsetState {
> +    /* Empty */
> +} CPUNegativeOffsetState;

It would be nice if we could do a compile-time assert that
 offsetof(CPUArchState, neg) + sizeof(CPUNegativeOffsetState)
  == offsetof(CPUArchState, env)

but I guess the possibility of within-struct padding
between the two structs makes that dubious. Or does our
generated code rely on there being no padding between neg
and env anyway ?

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 31/36] cpu: Introduce CPUNegativeOffsetState

[Richard Henderson]

On 4/29/19 7:43 AM, Peter Maydell wrote:
> It would be nice if we could do a compile-time assert that
>  offsetof(CPUArchState, neg) + sizeof(CPUNegativeOffsetState)
>   == offsetof(CPUArchState, env)
> 
> but I guess the possibility of within-struct padding
> between the two structs makes that dubious. Or does our
> generated code rely on there being no padding between neg
> and env anyway ?

It would be nice, but there *is* padding for some targets.


r~

Re: [Qemu-devel] [PATCH for-4.1 v2 31/36] cpu: Introduce CPUNegativeOffsetState

[Richard Henderson]

On 4/29/19 7:43 AM, Peter Maydell wrote:
> It would be nice if we could do a compile-time assert that
>  offsetof(CPUArchState, neg) + sizeof(CPUNegativeOffsetState)
>   == offsetof(CPUArchState, env)
> 
> but I guess the possibility of within-struct padding
> between the two structs makes that dubious. Or does our
> generated code rely on there being no padding between neg
> and env anyway ?

It would be nice, but there *is* padding for some targets.


r~

Re: [Qemu-devel] [PATCH for-4.1 v2 32/36] cpu: Move icount_decr to CPUNegativeOffsetState

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:28, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Amusingly, we had already ignored the comment to keep this value at the
> end of CPUState.  This restores the minimum negative offset from TCG_AREG0
> for code generation.
>
> For the couple of uses within qom/cpu.c, add a pointer from the CPUState
> object to the IcountDecr object within CPUNegativeOffsetState.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>


> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 45ef41ebb2..032a62672e 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -54,7 +54,7 @@ typedef struct SyncClocks {
>  #define MAX_DELAY_PRINT_RATE 2000000000LL
>  #define MAX_NB_PRINTS 100
>
> -static void align_clocks(SyncClocks *sc, const CPUState *cpu)
> +static void align_clocks(SyncClocks *sc, CPUState *cpu)
>  {
>      int64_t cpu_icount;
>
> @@ -62,7 +62,7 @@ static void align_clocks(SyncClocks *sc, const CPUState *cpu)
>          return;
>      }
>
> -    cpu_icount = cpu->icount_extra + cpu->icount_decr.u16.low;
> +    cpu_icount = cpu->icount_extra + cpu_neg(cpu)->icount_decr.u16.low;
>      sc->diff_clk += cpu_icount_to_ns(sc->last_cpu_icount - cpu_icount);
>      sc->last_cpu_icount = cpu_icount;

Why does this require that we remove the 'const' from the cpu
argument to the function ?


> @@ -1404,7 +1405,7 @@ static void process_icount_data(CPUState *cpu)
>          cpu_update_icount(cpu);
>
>          /* Reset the counters */
> -        cpu->icount_decr.u16.low = 0;
> +        cpu_neg(cpu)->icount_decr.u16.low = 0;
>          cpu->icount_extra = 0;
>          cpu->icount_budget = 0;
>


>  int cpu_write_elf32_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
> @@ -265,7 +265,7 @@ static void cpu_common_reset(CPUState *cpu)
>      cpu->mem_io_pc = 0;
>      cpu->mem_io_vaddr = 0;
>      cpu->icount_extra = 0;
> -    atomic_set(&cpu->icount_decr.u32, 0);
> +    atomic_set(&cpu->icount_decr_ptr->u32, 0);
>      cpu->can_do_io = 1;
>      cpu->exception_index = -1;
>      cpu->crash_occurred = false;

What determines when we need to access the icount fields
via icount_decr_ptr versus when we can just directly
access icount_decr ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 32/36] cpu: Move icount_decr to CPUNegativeOffsetState

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:28, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Amusingly, we had already ignored the comment to keep this value at the
> end of CPUState.  This restores the minimum negative offset from TCG_AREG0
> for code generation.
>
> For the couple of uses within qom/cpu.c, add a pointer from the CPUState
> object to the IcountDecr object within CPUNegativeOffsetState.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>


> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 45ef41ebb2..032a62672e 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -54,7 +54,7 @@ typedef struct SyncClocks {
>  #define MAX_DELAY_PRINT_RATE 2000000000LL
>  #define MAX_NB_PRINTS 100
>
> -static void align_clocks(SyncClocks *sc, const CPUState *cpu)
> +static void align_clocks(SyncClocks *sc, CPUState *cpu)
>  {
>      int64_t cpu_icount;
>
> @@ -62,7 +62,7 @@ static void align_clocks(SyncClocks *sc, const CPUState *cpu)
>          return;
>      }
>
> -    cpu_icount = cpu->icount_extra + cpu->icount_decr.u16.low;
> +    cpu_icount = cpu->icount_extra + cpu_neg(cpu)->icount_decr.u16.low;
>      sc->diff_clk += cpu_icount_to_ns(sc->last_cpu_icount - cpu_icount);
>      sc->last_cpu_icount = cpu_icount;

Why does this require that we remove the 'const' from the cpu
argument to the function ?


> @@ -1404,7 +1405,7 @@ static void process_icount_data(CPUState *cpu)
>          cpu_update_icount(cpu);
>
>          /* Reset the counters */
> -        cpu->icount_decr.u16.low = 0;
> +        cpu_neg(cpu)->icount_decr.u16.low = 0;
>          cpu->icount_extra = 0;
>          cpu->icount_budget = 0;
>


>  int cpu_write_elf32_qemunote(WriteCoreDumpFunction f, CPUState *cpu,
> @@ -265,7 +265,7 @@ static void cpu_common_reset(CPUState *cpu)
>      cpu->mem_io_pc = 0;
>      cpu->mem_io_vaddr = 0;
>      cpu->icount_extra = 0;
> -    atomic_set(&cpu->icount_decr.u32, 0);
> +    atomic_set(&cpu->icount_decr_ptr->u32, 0);
>      cpu->can_do_io = 1;
>      cpu->exception_index = -1;
>      cpu->crash_occurred = false;

What determines when we need to access the icount fields
via icount_decr_ptr versus when we can just directly
access icount_decr ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 32/36] cpu: Move icount_decr to CPUNegativeOffsetState

[Richard Henderson]

On 4/29/19 7:48 AM, Peter Maydell wrote:
>> -static void align_clocks(SyncClocks *sc, const CPUState *cpu)
>> +static void align_clocks(SyncClocks *sc, CPUState *cpu)
>>  {
>>      int64_t cpu_icount;
>>
>> @@ -62,7 +62,7 @@ static void align_clocks(SyncClocks *sc, const CPUState *cpu)
>>          return;
>>      }
>>
>> -    cpu_icount = cpu->icount_extra + cpu->icount_decr.u16.low;
>> +    cpu_icount = cpu->icount_extra + cpu_neg(cpu)->icount_decr.u16.low;
>>      sc->diff_clk += cpu_icount_to_ns(sc->last_cpu_icount - cpu_icount);
>>      sc->last_cpu_icount = cpu_icount;
> 
> Why does this require that we remove the 'const' from the cpu
> argument to the function ?

Because

  CPUNegativeOffsetState *cpu_neg(CPUState *);

and this isn't c++ so we can't overload with another

  const CPUNegativeOffsetState *cpu_neg(const CPUState *);

and it doesn't seem worthwhile to preserve constness here.

>> @@ -265,7 +265,7 @@ static void cpu_common_reset(CPUState *cpu)
>>      cpu->mem_io_pc = 0;
>>      cpu->mem_io_vaddr = 0;
>>      cpu->icount_extra = 0;
>> -    atomic_set(&cpu->icount_decr.u32, 0);
>> +    atomic_set(&cpu->icount_decr_ptr->u32, 0);
>>      cpu->can_do_io = 1;
>>      cpu->exception_index = -1;
>>      cpu->crash_occurred = false;
> 
> What determines when we need to access the icount fields
> via icount_decr_ptr versus when we can just directly
> access icount_decr ?

NEED_CPU_H.  To access the field directly you need ArchCPU defined.


r~

Re: [Qemu-devel] [PATCH for-4.1 v2 32/36] cpu: Move icount_decr to CPUNegativeOffsetState

[Richard Henderson]

On 4/29/19 7:48 AM, Peter Maydell wrote:
>> -static void align_clocks(SyncClocks *sc, const CPUState *cpu)
>> +static void align_clocks(SyncClocks *sc, CPUState *cpu)
>>  {
>>      int64_t cpu_icount;
>>
>> @@ -62,7 +62,7 @@ static void align_clocks(SyncClocks *sc, const CPUState *cpu)
>>          return;
>>      }
>>
>> -    cpu_icount = cpu->icount_extra + cpu->icount_decr.u16.low;
>> +    cpu_icount = cpu->icount_extra + cpu_neg(cpu)->icount_decr.u16.low;
>>      sc->diff_clk += cpu_icount_to_ns(sc->last_cpu_icount - cpu_icount);
>>      sc->last_cpu_icount = cpu_icount;
> 
> Why does this require that we remove the 'const' from the cpu
> argument to the function ?

Because

  CPUNegativeOffsetState *cpu_neg(CPUState *);

and this isn't c++ so we can't overload with another

  const CPUNegativeOffsetState *cpu_neg(const CPUState *);

and it doesn't seem worthwhile to preserve constness here.

>> @@ -265,7 +265,7 @@ static void cpu_common_reset(CPUState *cpu)
>>      cpu->mem_io_pc = 0;
>>      cpu->mem_io_vaddr = 0;
>>      cpu->icount_extra = 0;
>> -    atomic_set(&cpu->icount_decr.u32, 0);
>> +    atomic_set(&cpu->icount_decr_ptr->u32, 0);
>>      cpu->can_do_io = 1;
>>      cpu->exception_index = -1;
>>      cpu->crash_occurred = false;
> 
> What determines when we need to access the icount fields
> via icount_decr_ptr versus when we can just directly
> access icount_decr ?

NEED_CPU_H.  To access the field directly you need ArchCPU defined.


r~

Re: [Qemu-devel] [PATCH for-4.1 v2 34/36] cpu: Remove CPU_COMMON

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:25, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> This macro is now always empty, so remove it.  This leaves the
> entire contents of CPUArchState under the control of the guest
> architecture.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 34/36] cpu: Remove CPU_COMMON

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:25, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> This macro is now always empty, so remove it.  This leaves the
> entire contents of CPUArchState under the control of the guest
> architecture.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 35/36] tcg/aarch64: Use LDP to load tlb mask+table

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:29, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tcg/aarch64/tcg-target.inc.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/tcg/aarch64/tcg-target.inc.c b/tcg/aarch64/tcg-target.inc.c
> index ac765137ae..979efbcfe4 100644
> --- a/tcg/aarch64/tcg-target.inc.c
> +++ b/tcg/aarch64/tcg-target.inc.c
> @@ -1463,14 +1463,11 @@ static void tcg_out_tlb_read(TCGContext *s, TCGReg addr_reg, TCGMemOp opc,
>                               tcg_insn_unit **label_ptr, int mem_index,
>                               bool is_read)
>  {
> -    int fast_ofs = TLB_MASK_TABLE_OFS(mem_index);
> -    int mask_ofs = fast_ofs + offsetof(CPUTLBDescFast, mask);
> -    int table_ofs = fast_ofs + offsetof(CPUTLBDescFast, table);
>      unsigned a_bits = get_alignment_bits(opc);
>      unsigned s_bits = opc & MO_SIZE;
>      unsigned a_mask = (1u << a_bits) - 1;
>      unsigned s_mask = (1u << s_bits) - 1;
> -    TCGReg mask_base = TCG_AREG0, table_base = TCG_AREG0, x3;
> +    TCGReg x3;
>      TCGType mask_type;
>      uint64_t compare_mask;
>
> @@ -1478,8 +1475,8 @@ static void tcg_out_tlb_read(TCGContext *s, TCGReg addr_reg, TCGMemOp opc,
>                   ? TCG_TYPE_I64 : TCG_TYPE_I32);
>
>      /* Load tlb_mask[mmu_idx] and tlb_table[mmu_idx].  */

The field names in this comment are out of date, I think ?

> -    tcg_out_ld(s, mask_type, TCG_REG_X0, mask_base, mask_ofs);
> -    tcg_out_ld(s, TCG_TYPE_PTR, TCG_REG_X1, table_base, table_ofs);
> +    tcg_out_insn(s, 3314, LDP, TCG_REG_X0, TCG_REG_X1, TCG_AREG0,
> +                 TLB_MASK_TABLE_OFS(mem_index), 1, 0);

Can we have a compile time assert somewhere that the
mask and table fields are at the offsets in CPUTLBDescFast
that we expect them to be?

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 35/36] tcg/aarch64: Use LDP to load tlb mask+table

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:29, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tcg/aarch64/tcg-target.inc.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/tcg/aarch64/tcg-target.inc.c b/tcg/aarch64/tcg-target.inc.c
> index ac765137ae..979efbcfe4 100644
> --- a/tcg/aarch64/tcg-target.inc.c
> +++ b/tcg/aarch64/tcg-target.inc.c
> @@ -1463,14 +1463,11 @@ static void tcg_out_tlb_read(TCGContext *s, TCGReg addr_reg, TCGMemOp opc,
>                               tcg_insn_unit **label_ptr, int mem_index,
>                               bool is_read)
>  {
> -    int fast_ofs = TLB_MASK_TABLE_OFS(mem_index);
> -    int mask_ofs = fast_ofs + offsetof(CPUTLBDescFast, mask);
> -    int table_ofs = fast_ofs + offsetof(CPUTLBDescFast, table);
>      unsigned a_bits = get_alignment_bits(opc);
>      unsigned s_bits = opc & MO_SIZE;
>      unsigned a_mask = (1u << a_bits) - 1;
>      unsigned s_mask = (1u << s_bits) - 1;
> -    TCGReg mask_base = TCG_AREG0, table_base = TCG_AREG0, x3;
> +    TCGReg x3;
>      TCGType mask_type;
>      uint64_t compare_mask;
>
> @@ -1478,8 +1475,8 @@ static void tcg_out_tlb_read(TCGContext *s, TCGReg addr_reg, TCGMemOp opc,
>                   ? TCG_TYPE_I64 : TCG_TYPE_I32);
>
>      /* Load tlb_mask[mmu_idx] and tlb_table[mmu_idx].  */

The field names in this comment are out of date, I think ?

> -    tcg_out_ld(s, mask_type, TCG_REG_X0, mask_base, mask_ofs);
> -    tcg_out_ld(s, TCG_TYPE_PTR, TCG_REG_X1, table_base, table_ofs);
> +    tcg_out_insn(s, 3314, LDP, TCG_REG_X0, TCG_REG_X1, TCG_AREG0,
> +                 TLB_MASK_TABLE_OFS(mem_index), 1, 0);

Can we have a compile time assert somewhere that the
mask and table fields are at the offsets in CPUTLBDescFast
that we expect them to be?

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 36/36] tcg/arm: Use LDRD to load tlb mask+table

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:23, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tcg/arm/tcg-target.inc.c | 109 +++++++++++++++++++--------------------
>  1 file changed, 52 insertions(+), 57 deletions(-)

I think this would be easier to understand if the commit message
included what the old and new generated code fragments looked like.

(git diff seems to not have done a very good job of identifying
what's actually changed in this patch, unfortunately.)

The same remark about a compile-time check that mask and
table are where we expect from patch 35 applies here too.

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 36/36] tcg/arm: Use LDRD to load tlb mask+table

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:23, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tcg/arm/tcg-target.inc.c | 109 +++++++++++++++++++--------------------
>  1 file changed, 52 insertions(+), 57 deletions(-)

I think this would be easier to understand if the commit message
included what the old and new generated code fragments looked like.

(git diff seems to not have done a very good job of identifying
what's actually changed in this patch, unfortunately.)

The same remark about a compile-time check that mask and
table are where we expect from patch 35 applies here too.

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 36/36] tcg/arm: Use LDRD to load tlb mask+table

[Richard Henderson]

On 4/29/19 8:00 AM, Peter Maydell wrote:
> On Thu, 28 Mar 2019 at 23:23, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  tcg/arm/tcg-target.inc.c | 109 +++++++++++++++++++--------------------
>>  1 file changed, 52 insertions(+), 57 deletions(-)
> 
> I think this would be easier to understand if the commit message
> included what the old and new generated code fragments looked like.

Fair enough.  This change is complicated by having
to rearrange register usage, so that we ldrd into r0+r1
instead of ldr into r1+r2, etc.  Just this once it would
have been easier in thumb2.  ;-)

> The same remark about a compile-time check that mask and
> table are where we expect from patch 35 applies here too.

Ok.


r~

Re: [Qemu-devel] [PATCH for-4.1 v2 36/36] tcg/arm: Use LDRD to load tlb mask+table

[Richard Henderson]

On 4/29/19 8:00 AM, Peter Maydell wrote:
> On Thu, 28 Mar 2019 at 23:23, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  tcg/arm/tcg-target.inc.c | 109 +++++++++++++++++++--------------------
>>  1 file changed, 52 insertions(+), 57 deletions(-)
> 
> I think this would be easier to understand if the commit message
> included what the old and new generated code fragments looked like.

Fair enough.  This change is complicated by having
to rearrange register usage, so that we ldrd into r0+r1
instead of ldr into r1+r2, etc.  Just this once it would
have been easier in thumb2.  ;-)

> The same remark about a compile-time check that mask and
> table are where we expect from patch 35 applies here too.

Ok.


r~

Re: [Qemu-devel] [PATCH for-4.1 v2 33/36] cpu: Move the softmmu tlb to CPUNegativeOffsetState

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:26, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> We have for some time had code within the tcg backends to
> handle large positive offsets from env.  This move makes
> sure that need not happen.  Indeed, we are able to assert
> at build time that simple offsets suffice for all hosts.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 33/36] cpu: Move the softmmu tlb to CPUNegativeOffsetState

[Peter Maydell]

On Thu, 28 Mar 2019 at 23:26, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> We have for some time had code within the tcg backends to
> handle large positive offsets from env.  This move makes
> sure that need not happen.  Indeed, we are able to assert
> at build time that simple offsets suffice for all hosts.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 00/36] tcg: Move the softmmu tlb to CPUNegativeOffsetState

[Peter Maydell]

On Fri, 29 Mar 2019 at 00:44, <no-reply@patchew.org> wrote:
>
> Patchew URL: https://patchew.org/QEMU/20190328230404.12909-1-richard.henderson@linaro.org/
>
>
>
> Hi,
>
> This series seems to have some coding style problems. See output below for
> more information:

Richard -- at least some of these checkpatch nits look like
they're real rather than false positives: could you have a look
through them and fold in fixes where appropriate?

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.1 v2 00/36] tcg: Move the softmmu tlb to CPUNegativeOffsetState

[Peter Maydell]

On Fri, 29 Mar 2019 at 00:44, <no-reply@patchew.org> wrote:
>
> Patchew URL: https://patchew.org/QEMU/20190328230404.12909-1-richard.henderson@linaro.org/
>
>
>
> Hi,
>
> This series seems to have some coding style problems. See output below for
> more information:

Richard -- at least some of these checkpatch nits look like
they're real rather than false positives: could you have a look
through them and fold in fixes where appropriate?

thanks
-- PMM