Re: [PATCH v10 25/32] cpu: move do_unaligned_access to tcg_ops

[Claudio Fontana]

On 12/10/20 2:01 PM, Philippe Mathieu-Daudé wrote:
> On 12/10/20 1:12 PM, Claudio Fontana wrote:
>> make it consistently SOFTMMU-only.
>>
>> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  include/hw/core/cpu.h           | 17 +++--------------
>>  include/hw/core/tcg-cpu-ops.h   |  7 +++++++
>>  target/alpha/cpu.c              |  2 +-
>>  target/arm/cpu.c                |  2 +-
>>  target/hppa/cpu.c               |  4 +++-
>>  target/microblaze/cpu.c         |  2 +-
>>  target/mips/cpu.c               |  3 ++-
>>  target/nios2/cpu.c              |  2 +-
>>  target/riscv/cpu.c              |  2 +-
>>  target/s390x/cpu.c              |  2 +-
>>  target/sh4/cpu.c                |  2 +-
>>  target/sparc/cpu.c              |  2 +-
>>  target/xtensa/cpu.c             |  2 +-
>>  target/ppc/translate_init.c.inc |  2 +-
>>  14 files changed, 25 insertions(+), 26 deletions(-)
> ...
> 
>> -#ifdef CONFIG_SOFTMMU
>> +#if !defined(CONFIG_USER_ONLY) && defined(CONFIG_TCG)
>>  static inline void cpu_unaligned_access(CPUState *cpu, vaddr addr,
>>                                          MMUAccessType access_type,
>>                                          int mmu_idx, uintptr_t retaddr)
>>  {
>>      CPUClass *cc = CPU_GET_CLASS(cpu);
>>  
>> -    cc->do_unaligned_access(cpu, addr, access_type, mmu_idx, retaddr);
>> +    cc->tcg_ops.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,10 +850,7 @@ static inline void cpu_transaction_failed(CPUState 
>> *cpu, hwaddr physaddr,
>>                                            mmu_idx, attrs, response, 
>> retaddr);
>>      }
>>  }
>> -#endif /* CONFIG_TCG */
>> -#endif /* CONFIG_SOFTMMU */
>> -
>> -#endif /* NEED_CPU_H */
>> +#endif /* !CONFIG_USER_ONLY && CONFIG_TCG */
>>  
>>  /**
>>   * cpu_set_pc:
>> diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
>> index 3cc2733410..bac0165db6 100644
>> --- a/include/hw/core/tcg-cpu-ops.h
>> +++ b/include/hw/core/tcg-cpu-ops.h
>> @@ -50,6 +50,13 @@ typedef struct TcgCpuOperations {
>>                                    unsigned size, MMUAccessType access_type,
>>                                    int mmu_idx, MemTxAttrs attrs,
>>                                    MemTxResult response, uintptr_t retaddr);
>> +    /**
>> +     * @do_unaligned_access: Callback for unaligned access handling, if
>> +     * the target defines #TARGET_ALIGNED_ONLY.
>> +     */
>> +    void (*do_unaligned_access)(CPUState *cpu, vaddr addr,
>> +                                MMUAccessType access_type,
>> +                                int mmu_idx, uintptr_t retaddr);
> 
> Similarly to previous patch, don't we want to restrict this
> to system-mode?

In theory yes, (and what about exec_interrupt..?)
but we need to triple check the targets, because they tend to set these 
unconditionally.

Same concern about CONFIG_USER_ONLY vs NEED_CPU_H / CONFIG_SOFTMMU, would use 
CONFIG_USER_ONLY for consistency with the other targets?



> 
>>  
>>      /**
>>       * @tlb_fill: Handle a softmmu tlb miss or user-only address fault
>