On 12/11/20 6:10 PM, Claudio Fontana wrote: > On 12/11/20 6:05 PM, Richard Henderson wrote: >> On 12/11/20 2:31 AM, Claudio Fontana wrote: >>> From: Eduardo Habkost <ehabk...@redhat.com> >>> >>> Signed-off-by: Eduardo Habkost <ehabk...@redhat.com> >>> [claudio: wrapped in CONFIG_TCG] >>> Signed-off-by: Claudio Fontana <cfont...@suse.de> >>> Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com> >>> Reviewed-by: Alex Bennée <alex.ben...@linaro.org> >>> --- >>> include/hw/core/cpu.h | 8 -------- >>> include/hw/core/tcg-cpu-ops.h | 12 ++++++++++++ >>> accel/tcg/cpu-exec.c | 4 ++-- >>> target/arm/cpu.c | 4 +++- >>> target/avr/cpu.c | 2 +- >>> target/hppa/cpu.c | 2 +- >>> target/i386/tcg/tcg-cpu.c | 2 +- >>> target/microblaze/cpu.c | 2 +- >>> target/mips/cpu.c | 4 +++- >>> target/riscv/cpu.c | 2 +- >>> target/rx/cpu.c | 2 +- >>> target/sh4/cpu.c | 2 +- >>> target/sparc/cpu.c | 2 +- >>> target/tricore/cpu.c | 2 +- >>> 14 files changed, 29 insertions(+), 21 deletions(-) >>> >>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h >>> index ea648d52ad..83007d262c 100644 >>> --- a/include/hw/core/cpu.h >>> +++ b/include/hw/core/cpu.h >>> @@ -110,13 +110,6 @@ struct TranslationBlock; >>> * If the target behaviour here is anything other than "set >>> * the PC register to the value passed in" then the target must >>> * also implement the synchronize_from_tb hook. >>> - * @synchronize_from_tb: Callback for synchronizing state from a TCG >>> - * #TranslationBlock. This is called when we abandon execution >>> - * of a TB before starting it, and must set all parts of the CPU >>> - * state which the previous TB in the chain may not have updated. >>> - * This always includes at least the program counter; some targets >>> - * will need to do more. If this hook is not implemented then the >>> - * default is to call @set_pc(tb->pc). >>> * @tlb_fill: Callback for handling a softmmu tlb miss or user-only >>> * address fault. For system mode, if the access is valid, call >>> * tlb_set_page and return true; if the access is invalid, and >>> @@ -193,7 +186,6 @@ struct CPUClass { >>> void (*get_memory_mapping)(CPUState *cpu, MemoryMappingList *list, >>> Error **errp); >>> void (*set_pc)(CPUState *cpu, vaddr value); >>> - void (*synchronize_from_tb)(CPUState *cpu, struct TranslationBlock >>> *tb); >>> bool (*tlb_fill)(CPUState *cpu, vaddr address, int size, >>> MMUAccessType access_type, int mmu_idx, >>> bool probe, uintptr_t retaddr); >>> diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h >>> index 4475ef0996..e1d50b3c8b 100644 >>> --- a/include/hw/core/tcg-cpu-ops.h >>> +++ b/include/hw/core/tcg-cpu-ops.h >>> @@ -10,6 +10,8 @@ >>> #ifndef TCG_CPU_OPS_H >>> #define TCG_CPU_OPS_H >>> >>> +#include "hw/core/cpu.h" >> >> This include is circular. > > Yes, it's protected though, it was asked that way.
I mean, during the review. I personally would have preferred to avoid these as only cpu.h is including this for now. > >> >> Are you sure that splitting out hw/core/tcg-cpu-ops.h from hw/core/cpu.h in >> patch 15 is even useful? > > it avoids a huge #ifdef CONFIG_TCG > > >> >> Otherwise the actual code change looks ok. >> >> >> r~ >> >