On Fri, Aug 04, 2017 at 06:20:43PM +0100, Peter Maydell wrote:
> Currently we have a rather half-baked setup for allowing CPUs to
> generate exceptions on accesses to invalid memory: the CPU has a
> cpu_unassigned_access() hook which the memory system calls in
> unassigned_mem_write() and unassigned_mem_read() if the current_cpu
> pointer is non-NULL. This was originally designed before we
> implemented the MemTxResult type that allows memory operations to
> report a success or failure code, which is why the hook is called
> right at the bottom of the memory system. The major problem with
> this is that it means that the hook can be called even when the
> access was not actually done by the CPU: for instance if the CPU
> writes to a DMA engine register which causes the DMA engine to begin
> a transaction which has been set up by the guest to operate on
> invalid memory then this will casue the CPU to take an exception
> incorrectly. Another minor problem is that currently if a device
> returns a transaction error then this won't turn into a CPU exception
> at all.
>
> The right way to do this is to have allow the CPU to respond
> to memory system transaction failures at the point where the
> CPU specific code calls into the memory system.
>
> Define a new QOM CPU method and utility function
> cpu_transaction_failed() which is called in these cases.
> The functionality here overlaps with the existing
> cpu_unassigned_access() because individual target CPUs will
> need some work to convert them to the new system. When this
> transition is complete we can remove the old cpu_unassigned_access()
> code.
>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
>
> ---
> include/qom/cpu.h | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 25eefea..fc54d55 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -85,8 +85,10 @@ struct TranslationBlock;
> * @has_work: Callback for checking if there is work to do.
> * @do_interrupt: Callback for interrupt handling.
> * @do_unassigned_access: Callback for unassigned access handling.
> + * (this is deprecated: new targets should use do_transaction_failed instead)
> * @do_unaligned_access: Callback for unaligned access handling, if
> * the target defines #ALIGNED_ONLY.
> + * @do_transaction_failed: Callback for handling failed memory transactions
Looks OK but I wonder if there you might want to clarify that this is a
bus/slave failure and not a failure within the CPU (e.g not an MMU fault).
Anyway:
Reviewed-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
> * @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
> @@ -153,6 +155,10 @@ typedef 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);
> @@ -837,6 +843,21 @@ static inline void cpu_unaligned_access(CPUState *cpu,
> vaddr addr,
>
> cc->do_unaligned_access(cpu, addr, access_type, mmu_idx, retaddr);
> }
> +
> +static inline void cpu_transaction_failed(CPUState *cpu, hwaddr physaddr,
> + vaddr addr, unsigned size,
> + MMUAccessType access_type,
> + int mmu_idx, MemTxAttrs attrs,
> + MemTxResult response,
> + uintptr_t retaddr)
> +{
> + CPUClass *cc = CPU_GET_CLASS(cpu);
> +
> + if (cc->do_transaction_failed) {
> + cc->do_transaction_failed(cpu, physaddr, addr, size, access_type,
> + mmu_idx, attrs, response, retaddr);
> + }
> +}
> #endif
>
> #endif /* NEED_CPU_H */
> --
> 2.7.4
>
>
