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

[Philippe Mathieu-Daudé]

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