Message ID | 20201210121226.19822-25-cfontana@suse.de (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | i386 cleanup | expand |
On 12/10/20 1:12 PM, Claudio Fontana wrote: > Signed-off-by: Claudio Fontana <cfontana@suse.de> > Reviewed-by: Alex Bennée <alex.bennee@linaro.org> > --- > include/hw/core/cpu.h | 18 +++++++----------- > include/hw/core/tcg-cpu-ops.h | 10 ++++++++++ > hw/mips/jazz.c | 9 +++++++-- > target/alpha/cpu.c | 2 +- > target/arm/cpu.c | 4 ++-- > target/m68k/cpu.c | 2 +- > target/microblaze/cpu.c | 2 +- > target/mips/cpu.c | 2 +- > target/riscv/cpu.c | 2 +- > target/sparc/cpu.c | 2 +- > target/xtensa/cpu.c | 2 +- > 11 files changed, 33 insertions(+), 22 deletions(-) ... > diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h > index a7387b5c39..3cc2733410 100644 > --- a/include/hw/core/tcg-cpu-ops.h > +++ b/include/hw/core/tcg-cpu-ops.h > @@ -11,6 +11,7 @@ > #define TCG_CPU_OPS_H > > #include "hw/core/cpu.h" > +#include "exec/memattrs.h" > > /** > * struct TcgCpuOperations: TCG operations specific to a CPU class > @@ -41,6 +42,15 @@ typedef struct TcgCpuOperations { > /** @do_interrupt: Callback for interrupt handling. */ > void (*do_interrupt)(CPUState *cpu); > Do we want to restrict this handler to system-mode? #ifdef CONFIG_SOFTMMU > + /** > + * @do_transaction_failed: Callback for handling failed memory transactions > + * (ie bus faults or external aborts; not MMU faults) > + */ > + void (*do_transaction_failed)(CPUState *cpu, hwaddr physaddr, vaddr addr, > + unsigned size, MMUAccessType access_type, > + int mmu_idx, MemTxAttrs attrs, > + MemTxResult response, uintptr_t retaddr); #endif > /** > * @tlb_fill: Handle a softmmu tlb miss or user-only address fault > * ...
On 12/10/20 1:59 PM, Philippe Mathieu-Daudé wrote: > On 12/10/20 1:12 PM, Claudio Fontana wrote: >> Signed-off-by: Claudio Fontana <cfontana@suse.de> >> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> >> --- >> include/hw/core/cpu.h | 18 +++++++----------- >> include/hw/core/tcg-cpu-ops.h | 10 ++++++++++ >> hw/mips/jazz.c | 9 +++++++-- >> target/alpha/cpu.c | 2 +- >> target/arm/cpu.c | 4 ++-- >> target/m68k/cpu.c | 2 +- >> target/microblaze/cpu.c | 2 +- >> target/mips/cpu.c | 2 +- >> target/riscv/cpu.c | 2 +- >> target/sparc/cpu.c | 2 +- >> target/xtensa/cpu.c | 2 +- >> 11 files changed, 33 insertions(+), 22 deletions(-) > ... > >> diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h >> index a7387b5c39..3cc2733410 100644 >> --- a/include/hw/core/tcg-cpu-ops.h >> +++ b/include/hw/core/tcg-cpu-ops.h >> @@ -11,6 +11,7 @@ >> #define TCG_CPU_OPS_H >> >> #include "hw/core/cpu.h" >> +#include "exec/memattrs.h" >> >> /** >> * struct TcgCpuOperations: TCG operations specific to a CPU class >> @@ -41,6 +42,15 @@ typedef struct TcgCpuOperations { >> /** @do_interrupt: Callback for interrupt handling. */ >> void (*do_interrupt)(CPUState *cpu); >> > > Do we want to restrict this handler to system-mode? maybe we should check them all... do_interrupt is sometimes also user mode, but do_transaction_failed seems to be system only. > > #ifdef CONFIG_SOFTMMU This could be either #ifndef CONFIG_USER_ONLY or #ifdef NEED_CPU_H #define CONFIG_SOFTMMU as discussed previously (Richard), issue is, in the header files here we tend to see #ifdef NEED_CPU_H #define CONFIG_SOFTMMU while the target/ files we consistently see #ifndef CONFIG_USER_ONLY so this is an inconsistency. If you are ok with it I would wrap everywhere consistently with CONFIG_USER_ONLY for now? >> + /** >> + * @do_transaction_failed: Callback for handling failed memory transactions >> + * (ie bus faults or external aborts; not MMU faults) >> + */ >> + void (*do_transaction_failed)(CPUState *cpu, hwaddr physaddr, vaddr addr, >> + unsigned size, MMUAccessType access_type, >> + int mmu_idx, MemTxAttrs attrs, >> + MemTxResult response, uintptr_t retaddr); > > #endif > >> /** >> * @tlb_fill: Handle a softmmu tlb miss or user-only address fault >> * > ... >
On 12/10/20 2:10 PM, Claudio Fontana wrote: > On 12/10/20 1:59 PM, Philippe Mathieu-Daudé wrote: >> On 12/10/20 1:12 PM, Claudio Fontana wrote: >>> Signed-off-by: Claudio Fontana <cfontana@suse.de> >>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> >>> --- >>> include/hw/core/cpu.h | 18 +++++++----------- >>> include/hw/core/tcg-cpu-ops.h | 10 ++++++++++ >>> hw/mips/jazz.c | 9 +++++++-- >>> target/alpha/cpu.c | 2 +- >>> target/arm/cpu.c | 4 ++-- >>> target/m68k/cpu.c | 2 +- >>> target/microblaze/cpu.c | 2 +- >>> target/mips/cpu.c | 2 +- >>> target/riscv/cpu.c | 2 +- >>> target/sparc/cpu.c | 2 +- >>> target/xtensa/cpu.c | 2 +- >>> 11 files changed, 33 insertions(+), 22 deletions(-) >> ... >> >>> diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h >>> index a7387b5c39..3cc2733410 100644 >>> --- a/include/hw/core/tcg-cpu-ops.h >>> +++ b/include/hw/core/tcg-cpu-ops.h >>> @@ -11,6 +11,7 @@ >>> #define TCG_CPU_OPS_H >>> >>> #include "hw/core/cpu.h" >>> +#include "exec/memattrs.h" >>> >>> /** >>> * struct TcgCpuOperations: TCG operations specific to a CPU class >>> @@ -41,6 +42,15 @@ typedef struct TcgCpuOperations { >>> /** @do_interrupt: Callback for interrupt handling. */ >>> void (*do_interrupt)(CPUState *cpu); >>> >> >> Do we want to restrict this handler to system-mode? > > maybe we should check them all... do_interrupt is sometimes also user mode, > but do_transaction_failed seems to be system only. Ah, signal handlers maybe. > >> >> #ifdef CONFIG_SOFTMMU > > This could be either > > #ifndef CONFIG_USER_ONLY > > or > > #ifdef NEED_CPU_H > #define CONFIG_SOFTMMU Yes, whatever, this is the same for now ... > > as discussed previously (Richard), > > issue is, in the header files here we tend to see > > #ifdef NEED_CPU_H > #define CONFIG_SOFTMMU > > while the target/ files we consistently see > > #ifndef CONFIG_USER_ONLY ... until we sanitize it as suggested. > > > so this is an inconsistency. > > If you are ok with it I would wrap everywhere consistently with CONFIG_USER_ONLY for now? > > >>> + /** >>> + * @do_transaction_failed: Callback for handling failed memory transactions >>> + * (ie bus faults or external aborts; not MMU faults) >>> + */ >>> + void (*do_transaction_failed)(CPUState *cpu, hwaddr physaddr, vaddr addr, >>> + unsigned size, MMUAccessType access_type, >>> + int mmu_idx, MemTxAttrs attrs, >>> + MemTxResult response, uintptr_t retaddr); >> >> #endif >> >>> /** >>> * @tlb_fill: Handle a softmmu tlb miss or user-only address fault >>> * >> ... >> >
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index b178549d48..0c6c17fb48 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -89,8 +89,6 @@ struct TranslationBlock; * @has_work: Callback for checking if there is work to do. * @do_unaligned_access: Callback for unaligned access handling, if * the target defines #TARGET_ALIGNED_ONLY. - * @do_transaction_failed: Callback for handling failed memory transactions - * (ie bus faults or external aborts; not MMU faults) * @virtio_is_big_endian: Callback to return %true if a CPU which supports * runtime configurable endianness is currently big-endian. Non-configurable * CPUs can use the default implementation of this method. This method should @@ -159,10 +157,6 @@ struct CPUClass { void (*do_unaligned_access)(CPUState *cpu, vaddr addr, MMUAccessType access_type, int mmu_idx, uintptr_t retaddr); - void (*do_transaction_failed)(CPUState *cpu, hwaddr physaddr, vaddr addr, - unsigned size, MMUAccessType access_type, - int mmu_idx, MemTxAttrs attrs, - MemTxResult response, uintptr_t retaddr); bool (*virtio_is_big_endian)(CPUState *cpu); int (*memory_rw_debug)(CPUState *cpu, vaddr addr, uint8_t *buf, int len, bool is_write); @@ -848,7 +842,7 @@ static inline void cpu_unaligned_access(CPUState *cpu, vaddr addr, cc->do_unaligned_access(cpu, addr, access_type, mmu_idx, retaddr); } - +#ifdef CONFIG_TCG static inline void cpu_transaction_failed(CPUState *cpu, hwaddr physaddr, vaddr addr, unsigned size, MMUAccessType access_type, @@ -858,12 +852,14 @@ static inline void cpu_transaction_failed(CPUState *cpu, hwaddr physaddr, { CPUClass *cc = CPU_GET_CLASS(cpu); - if (!cpu->ignore_memory_transaction_failures && cc->do_transaction_failed) { - cc->do_transaction_failed(cpu, physaddr, addr, size, access_type, - mmu_idx, attrs, response, retaddr); + if (!cpu->ignore_memory_transaction_failures && + cc->tcg_ops.do_transaction_failed) { + cc->tcg_ops.do_transaction_failed(cpu, physaddr, addr, size, access_type, + mmu_idx, attrs, response, retaddr); } } -#endif +#endif /* CONFIG_TCG */ +#endif /* CONFIG_SOFTMMU */ #endif /* NEED_CPU_H */ diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h index a7387b5c39..3cc2733410 100644 --- a/include/hw/core/tcg-cpu-ops.h +++ b/include/hw/core/tcg-cpu-ops.h @@ -11,6 +11,7 @@ #define TCG_CPU_OPS_H #include "hw/core/cpu.h" +#include "exec/memattrs.h" /** * struct TcgCpuOperations: TCG operations specific to a CPU class @@ -41,6 +42,15 @@ typedef struct TcgCpuOperations { /** @do_interrupt: Callback for interrupt handling. */ void (*do_interrupt)(CPUState *cpu); + /** + * @do_transaction_failed: Callback for handling failed memory transactions + * (ie bus faults or external aborts; not MMU faults) + */ + void (*do_transaction_failed)(CPUState *cpu, hwaddr physaddr, vaddr addr, + unsigned size, MMUAccessType access_type, + int mmu_idx, MemTxAttrs attrs, + MemTxResult response, uintptr_t retaddr); + /** * @tlb_fill: Handle a softmmu tlb miss or user-only address fault * diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c index 71448f72ac..2745ba3906 100644 --- a/hw/mips/jazz.c +++ b/hw/mips/jazz.c @@ -115,6 +115,8 @@ static const MemoryRegionOps dma_dummy_ops = { #define MAGNUM_BIOS_SIZE_MAX 0x7e000 #define MAGNUM_BIOS_SIZE \ (BIOS_SIZE < MAGNUM_BIOS_SIZE_MAX ? BIOS_SIZE : MAGNUM_BIOS_SIZE_MAX) + +#ifdef CONFIG_TCG static void (*real_do_transaction_failed)(CPUState *cpu, hwaddr physaddr, vaddr addr, unsigned size, MMUAccessType access_type, @@ -136,6 +138,7 @@ static void mips_jazz_do_transaction_failed(CPUState *cs, hwaddr physaddr, (*real_do_transaction_failed)(cs, physaddr, addr, size, access_type, mmu_idx, attrs, response, retaddr); } +#endif /* CONFIG_TCG */ static void mips_jazz_init(MachineState *machine, enum jazz_model_e jazz_model) @@ -204,8 +207,10 @@ static void mips_jazz_init(MachineState *machine, * memory region that catches all memory accesses, as we do on Malta. */ cc = CPU_GET_CLASS(cpu); - real_do_transaction_failed = cc->do_transaction_failed; - cc->do_transaction_failed = mips_jazz_do_transaction_failed; +#ifdef CONFIG_TCG + real_do_transaction_failed = cc->tcg_ops.do_transaction_failed; + cc->tcg_ops.do_transaction_failed = mips_jazz_do_transaction_failed; +#endif /* CONFIG_TCG */ /* allocate RAM */ memory_region_add_subregion(address_space, 0, machine->ram); diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c index fde5825ebc..3e651b246f 100644 --- a/target/alpha/cpu.c +++ b/target/alpha/cpu.c @@ -224,7 +224,7 @@ static void alpha_cpu_class_init(ObjectClass *oc, void *data) cc->gdb_write_register = alpha_cpu_gdb_write_register; cc->tcg_ops.tlb_fill = alpha_cpu_tlb_fill; #ifndef CONFIG_USER_ONLY - cc->do_transaction_failed = alpha_cpu_do_transaction_failed; + cc->tcg_ops.do_transaction_failed = alpha_cpu_do_transaction_failed; cc->do_unaligned_access = alpha_cpu_do_unaligned_access; cc->get_phys_page_debug = alpha_cpu_get_phys_page_debug; dc->vmsd = &vmstate_alpha_cpu; diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 826cf8351b..5e9202ed86 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -2268,11 +2268,11 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data) cc->debug_check_watchpoint = arm_debug_check_watchpoint; cc->do_unaligned_access = arm_cpu_do_unaligned_access; #if !defined(CONFIG_USER_ONLY) - cc->do_transaction_failed = arm_cpu_do_transaction_failed; + cc->tcg_ops.do_transaction_failed = arm_cpu_do_transaction_failed; cc->adjust_watchpoint_address = arm_adjust_watchpoint_address; cc->tcg_ops.do_interrupt = arm_cpu_do_interrupt; #endif /* CONFIG_TCG && !CONFIG_USER_ONLY */ -#endif +#endif /* CONFIG_TCG */ } #ifdef CONFIG_KVM diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c index 3e84de772c..3f60c99865 100644 --- a/target/m68k/cpu.c +++ b/target/m68k/cpu.c @@ -285,7 +285,7 @@ static void m68k_cpu_class_init(ObjectClass *c, void *data) cc->gdb_write_register = m68k_cpu_gdb_write_register; cc->tcg_ops.tlb_fill = m68k_cpu_tlb_fill; #if defined(CONFIG_SOFTMMU) - cc->do_transaction_failed = m68k_cpu_transaction_failed; + cc->tcg_ops.do_transaction_failed = m68k_cpu_transaction_failed; cc->get_phys_page_debug = m68k_cpu_get_phys_page_debug; #endif cc->disas_set_info = m68k_cpu_disas_set_info; diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c index 4f95248b2e..fa57a324dc 100644 --- a/target/microblaze/cpu.c +++ b/target/microblaze/cpu.c @@ -327,7 +327,7 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data) cc->gdb_write_register = mb_cpu_gdb_write_register; cc->tcg_ops.tlb_fill = mb_cpu_tlb_fill; #ifndef CONFIG_USER_ONLY - cc->do_transaction_failed = mb_cpu_transaction_failed; + cc->tcg_ops.do_transaction_failed = mb_cpu_transaction_failed; cc->get_phys_page_debug = mb_cpu_get_phys_page_debug; dc->vmsd = &vmstate_mb_cpu; #endif diff --git a/target/mips/cpu.c b/target/mips/cpu.c index dd5fb78e6c..259bb791f7 100644 --- a/target/mips/cpu.c +++ b/target/mips/cpu.c @@ -239,7 +239,6 @@ static void mips_cpu_class_init(ObjectClass *c, void *data) cc->gdb_read_register = mips_cpu_gdb_read_register; cc->gdb_write_register = mips_cpu_gdb_write_register; #ifndef CONFIG_USER_ONLY - cc->do_transaction_failed = mips_cpu_do_transaction_failed; cc->do_unaligned_access = mips_cpu_do_unaligned_access; cc->get_phys_page_debug = mips_cpu_get_phys_page_debug; cc->vmsd = &vmstate_mips_cpu; @@ -252,6 +251,7 @@ static void mips_cpu_class_init(ObjectClass *c, void *data) cc->tcg_ops.tlb_fill = mips_cpu_tlb_fill; #ifndef CONFIG_USER_ONLY cc->tcg_ops.do_interrupt = mips_cpu_do_interrupt; + cc->tcg_ops.do_transaction_failed = mips_cpu_do_transaction_failed; #endif /* CONFIG_USER_ONLY */ #endif /* CONFIG_TCG */ diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index 6a815569cc..7dfd8d7339 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -555,7 +555,7 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data) cc->gdb_stop_before_watchpoint = true; cc->disas_set_info = riscv_cpu_disas_set_info; #ifndef CONFIG_USER_ONLY - cc->do_transaction_failed = riscv_cpu_do_transaction_failed; + cc->tcg_ops.do_transaction_failed = riscv_cpu_do_transaction_failed; cc->do_unaligned_access = riscv_cpu_do_unaligned_access; cc->get_phys_page_debug = riscv_cpu_get_phys_page_debug; /* For now, mark unmigratable: */ diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c index 3f23aa5962..009d0f07c3 100644 --- a/target/sparc/cpu.c +++ b/target/sparc/cpu.c @@ -873,7 +873,7 @@ static void sparc_cpu_class_init(ObjectClass *oc, void *data) cc->gdb_write_register = sparc_cpu_gdb_write_register; cc->tcg_ops.tlb_fill = sparc_cpu_tlb_fill; #ifndef CONFIG_USER_ONLY - cc->do_transaction_failed = sparc_cpu_do_transaction_failed; + cc->tcg_ops.do_transaction_failed = sparc_cpu_do_transaction_failed; cc->do_unaligned_access = sparc_cpu_do_unaligned_access; cc->get_phys_page_debug = sparc_cpu_get_phys_page_debug; cc->vmsd = &vmstate_sparc_cpu; diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c index 3ff025f0fe..fc52fde696 100644 --- a/target/xtensa/cpu.c +++ b/target/xtensa/cpu.c @@ -205,7 +205,7 @@ static void xtensa_cpu_class_init(ObjectClass *oc, void *data) #ifndef CONFIG_USER_ONLY cc->do_unaligned_access = xtensa_cpu_do_unaligned_access; cc->get_phys_page_debug = xtensa_cpu_get_phys_page_debug; - cc->do_transaction_failed = xtensa_cpu_do_transaction_failed; + cc->tcg_ops.do_transaction_failed = xtensa_cpu_do_transaction_failed; #endif cc->tcg_ops.debug_excp_handler = xtensa_breakpoint_handler; cc->disas_set_info = xtensa_cpu_disas_set_info;