Re: [RFC PATCH 3/4] accel/tcg: Guard tb_flush() with tcg_enabled() and remove the stub

[Harsh Prateek Bora <harshpb@linux.ibm.com>]

On 9/15/23 01:22, Philippe Mathieu-Daudé wrote:
> The check on tcg_enabled() make it clearer we want
> this call under TCG.
> 

tb_flush already has a check for tcg_enabled() in its definition.
Do we really need to check for same before calling it?

Thanks
Harsh

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   accel/stubs/tcg-stub.c |  4 ----
>   cpu.c                  | 15 +++++++++------
>   gdbstub/softmmu.c      |  5 ++++-
>   hw/ppc/spapr_hcall.c   |  2 +-
>   4 files changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/accel/stubs/tcg-stub.c b/accel/stubs/tcg-stub.c
> index dd890d6cf6..7d9846f7f2 100644
> --- a/accel/stubs/tcg-stub.c
> +++ b/accel/stubs/tcg-stub.c
> @@ -14,10 +14,6 @@
>   #include "exec/tb-flush.h"
>   #include "exec/exec-all.h"
>   
> -void tb_flush(CPUState *cpu)
> -{
> -}
> -
>   int probe_access_flags(CPUArchState *env, vaddr addr, int size,
>                          MMUAccessType access_type, int mmu_idx,
>                          bool nonfault, void **phost, uintptr_t retaddr)
> diff --git a/cpu.c b/cpu.c
> index 0769b0b153..ce3b04f21f 100644
> --- a/cpu.c
> +++ b/cpu.c
> @@ -57,12 +57,15 @@ static int cpu_common_post_load(void *opaque, int version_id)
>       cpu->interrupt_request &= ~0x01;
>       tlb_flush(cpu);
>   
> -    /* loadvm has just updated the content of RAM, bypassing the
> -     * usual mechanisms that ensure we flush TBs for writes to
> -     * memory we've translated code from. So we must flush all TBs,
> -     * which will now be stale.
> -     */
> -    tb_flush(cpu);
> +    if (tcg_enabled()) {
> +        /*
> +         * loadvm has just updated the content of RAM, bypassing the
> +         * usual mechanisms that ensure we flush TBs for writes to
> +         * memory we've translated code from. So we must flush all TBs,
> +         * which will now be stale.
> +         */
> +        tb_flush(cpu);
> +    }
>   
>       return 0;
>   }
> diff --git a/gdbstub/softmmu.c b/gdbstub/softmmu.c
> index 9f0b8b5497..edd13f047d 100644
> --- a/gdbstub/softmmu.c
> +++ b/gdbstub/softmmu.c
> @@ -21,6 +21,7 @@
>   #include "sysemu/cpus.h"
>   #include "sysemu/runstate.h"
>   #include "sysemu/replay.h"
> +#include "sysemu/tcg.h"
>   #include "hw/core/cpu.h"
>   #include "hw/cpu/cluster.h"
>   #include "hw/boards.h"
> @@ -170,7 +171,9 @@ static void gdb_vm_state_change(void *opaque, bool running, RunState state)
>           } else {
>               trace_gdbstub_hit_break();
>           }
> -        tb_flush(cpu);
> +        if (tcg_enabled()) {
> +            tb_flush(cpu);
> +        }
>           ret = GDB_SIGNAL_TRAP;
>           break;
>       case RUN_STATE_PAUSED:
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index b7dc388f2f..306f8fdf55 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -296,7 +296,7 @@ static target_ulong h_page_init(PowerPCCPU *cpu, SpaprMachineState *spapr,
>       if (flags & (H_ICACHE_SYNCHRONIZE | H_ICACHE_INVALIDATE)) {
>           if (kvm_enabled()) {
>               kvmppc_icbi_range(cpu, pdst, len);
> -        } else {
> +        } else if (tcg_enabled()) {
>               tb_flush(CPU(cpu));
>           }
>       }