qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-arm] [PATCH v3 1/2] cpu: Add callback to check architectural w


From: Peter Maydell
Subject: Re: [Qemu-arm] [PATCH v3 1/2] cpu: Add callback to check architectural watchpoint match
Date: Tue, 2 Feb 2016 12:23:46 +0000

On 31 January 2016 at 16:15, Sergey Fedorov <address@hidden> wrote:
> When QEMU watchpoint matches, that is not definitely an architectural
> watchpoint match yet. If it is a stop-before-access watchpoint then that
> is hardly possible to ignore it after throwing a TCG exception.
>
> A special callback is introduced to check for architectural watchpoint
> match before raising a TCG exception.
>
> Signed-off-by: Sergey Fedorov <address@hidden>

This looks OK to me, but there's one QOM CPU style question
I'd like Andreas's view on.

> @@ -2024,6 +2024,7 @@ static const MemoryRegionOps notdirty_mem_ops = {
>  static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int 
> flags)
>  {
>      CPUState *cpu = current_cpu;
> +    CPUClass *cc = CPU_GET_CLASS(cpu);
>      CPUArchState *env = cpu->env_ptr;
>      target_ulong pc, cs_base;
>      target_ulong vaddr;
> @@ -2049,6 +2050,11 @@ static void check_watchpoint(int offset, int len, 
> MemTxAttrs attrs, int flags)
>              wp->hitaddr = vaddr;
>              wp->hitattrs = attrs;
>              if (!cpu->watchpoint_hit) {
> +                if (wp->flags & BP_CPU &&
> +                    !cc->debug_check_watchpoint(cpu, wp)) {

At least some of the QOM CPU methods have wrapper functions
(eg cpu_set_pc(), cpu_unaligned_access()) that just bundle up
the CPU_GET_CLASS and method invocation). Should new methods
like the debug_check_watchpoint() introduced by this patch have
that kind of wrapper function, or is it optional?

(There also seems to be a mix of "implement default/common
behaviour in a common method implementation" vs "implement
it in the wrapper function if the method pointer is NULL".
I think the former, as done in this patch, is probably nicer
but again would defer to Andreas.)

thanks
-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]