Re: [PATCH v10 24/32] cpu: move cc->transaction_failed to tcg_ops

[Claudio Fontana]

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
>>       *
> ...
>