Re: [Qemu-devel] [PATCH 02/13] Refactor and enhance break/watchpoint API

[Glauber Costa]

>  /* Add a watchpoint.  */
> -int cpu_watchpoint_insert(CPUState *env, target_ulong addr, int type)
> +int cpu_watchpoint_insert(CPUState *env, target_ulong addr, target_ulong len,
> +                          int flags, CPUWatchpoint **watchpoint)

Any particular reason you're not simply returning the address of the watchpoint?
Although I must admit it's a matter of personal taste.

>  {
> -    int i;
> +    CPUWatchpoint *wp;
>
> -    for (i = 0; i < env->nb_watchpoints; i++) {
> -        if (addr == env->watchpoint[i].vaddr)
> -            return 0;
> -    }
> -    if (env->nb_watchpoints >= MAX_WATCHPOINTS)
> -        return -1;
> +    wp = qemu_malloc(sizeof(*wp));
> +    if (!wp)
> +        return -ENOBUFS;
> +
> +    wp->vaddr = addr;
> +    wp->len = len;
> +    wp->flags = flags;
> +
> +    wp->next = env->watchpoints;
> +    wp->prev = NULL;
> +    if (wp->next)
> +        wp->next->prev = wp;
> +    env->watchpoints = wp;
>
> -    i = env->nb_watchpoints++;
> -    env->watchpoint[i].vaddr = addr;
> -    env->watchpoint[i].type = type;
>     tlb_flush_page(env, addr);
>     /* FIXME: This flush is needed because of the hack to make memory ops
>        terminate the TB.  It can be removed once the proper IO trap and
>        re-execute bits are in.  */
>     tb_flush(env);
> -    return i;
> -}
>
> -/* Remove a watchpoint.  */
> -int cpu_watchpoint_remove(CPUState *env, target_ulong addr)
> -{
> -    int i;
> +    if (watchpoint)
> +        *watchpoint = wp;
> +    return 0;
> +}
>
> -    for (i = 0; i < env->nb_watchpoints; i++) {
> -        if (addr == env->watchpoint[i].vaddr) {
> -            env->nb_watchpoints--;
> -            env->watchpoint[i] = env->watchpoint[env->nb_watchpoints];
> -            tlb_flush_page(env, addr);
> +/* Remove a specific watchpoint.  */
> +int cpu_watchpoint_remove(CPUState *env, target_ulong addr, target_ulong len,
> +                          int flags)
> +{
> +    CPUWatchpoint *wp;
> +
> +    for (wp = env->watchpoints; wp != NULL; wp = wp->next) {
> +        if (addr == wp->vaddr && len == wp->len && flags == wp->flags) {
> +            cpu_watchpoint_remove_by_ref(env, wp);
>             return 0;
>         }
>     }
> -    return -1;
> +    return -ENOENT;
>  }
>
> -/* Remove all watchpoints. */
> -void cpu_watchpoint_remove_all(CPUState *env) {
> -    int i;
> +/* Remove a specific watchpoint by reference.  */
> +void cpu_watchpoint_remove_by_ref(CPUState *env, CPUWatchpoint *watchpoint)
> +{
> +    if (watchpoint->next)
> +        watchpoint->next->prev = watchpoint->prev;
> +    if (watchpoint->prev)
> +        watchpoint->prev->next = watchpoint->next;
> +    else
> +        env->watchpoints = watchpoint->next;
>
> -    for (i = 0; i < env->nb_watchpoints; i++) {
> -        tlb_flush_page(env, env->watchpoint[i].vaddr);
> -    }
> -    env->nb_watchpoints = 0;
> +    tlb_flush_page(env, watchpoint->vaddr);
> +
> +    qemu_free(watchpoint);
>  }
>
> -/* add a breakpoint. EXCP_DEBUG is returned by the CPU loop if a
> -   breakpoint is reached */
> -int cpu_breakpoint_insert(CPUState *env, target_ulong pc)
> +/* Remove all matching watchpoints.  */
> +void cpu_watchpoint_remove_all(CPUState *env, int mask)
>  {
> -#if defined(TARGET_HAS_ICE)
> -    int i;
> +    CPUWatchpoint *wp;
>
> -    for(i = 0; i < env->nb_breakpoints; i++) {
> -        if (env->breakpoints[i] == pc)
> -            return 0;
> -    }
> +    for (wp = env->watchpoints; wp != NULL; wp = wp->next)
> +        if (wp->flags & mask)
> +            cpu_watchpoint_remove_by_ref(env, wp);
> +}
>
> -    if (env->nb_breakpoints >= MAX_BREAKPOINTS)
> -        return -1;
> -    env->breakpoints[env->nb_breakpoints++] = pc;
> +/* Add a breakpoint.  */
> +int cpu_breakpoint_insert(CPUState *env, target_ulong pc, int flags,
> +                          CPUBreakpoint **breakpoint)
> +{
> +#if defined(TARGET_HAS_ICE)
> +    CPUBreakpoint *bp;
> +
> +    bp = qemu_malloc(sizeof(*bp));
> +    if (!bp)
> +        return -ENOBUFS;
> +
> +    bp->pc = pc;
> +    bp->flags = flags;
> +
> +    bp->next = env->breakpoints;
> +    bp->prev = NULL;
> +    if (bp->next)
> +        bp->next->prev = bp;
> +    env->breakpoints = bp;
>
>     breakpoint_invalidate(env, pc);
> +
> +    if (breakpoint)
> +        *breakpoint = bp;
>     return 0;
>  #else
> -    return -1;
> +    return -ENOSYS;
>  #endif
>  }
>
> -/* remove all breakpoints */
> -void cpu_breakpoint_remove_all(CPUState *env) {
> +/* Remove a specific breakpoint.  */
> +int cpu_breakpoint_remove(CPUState *env, target_ulong pc, int flags)
> +{
>  #if defined(TARGET_HAS_ICE)
> -    int i;
> -    for(i = 0; i < env->nb_breakpoints; i++) {
> -        breakpoint_invalidate(env, env->breakpoints[i]);
> +    CPUBreakpoint *bp;
> +
> +    for (bp = env->breakpoints; bp != NULL; bp = bp->next) {
> +        if (bp->pc == pc && bp->flags == flags) {
> +            cpu_breakpoint_remove_by_ref(env, bp);
> +            return 0;
> +        }
>     }
> -    env->nb_breakpoints = 0;
> +    return -ENOENT;
> +#else
> +    return -ENOSYS;
>  #endif
>  }
>
> -/* remove a breakpoint */
> -int cpu_breakpoint_remove(CPUState *env, target_ulong pc)
> +/* Remove a specific breakpoint by reference.  */
> +void cpu_breakpoint_remove_by_ref(CPUState *env, CPUBreakpoint *breakpoint)
>  {
>  #if defined(TARGET_HAS_ICE)
> -    int i;
> -    for(i = 0; i < env->nb_breakpoints; i++) {
> -        if (env->breakpoints[i] == pc)
> -            goto found;
> -    }
> -    return -1;
> - found:
> -    env->nb_breakpoints--;
> -    if (i < env->nb_breakpoints)
> -      env->breakpoints[i] = env->breakpoints[env->nb_breakpoints];
> +    if (breakpoint->next)
> +        breakpoint->next->prev = breakpoint->prev;
> +    if (breakpoint->prev)
> +        breakpoint->prev->next = breakpoint->next;
> +    else
> +        env->breakpoints = breakpoint->next;
>
> -    breakpoint_invalidate(env, pc);
> -    return 0;
> -#else
> -    return -1;
> +    breakpoint_invalidate(env, breakpoint->pc);
> +
> +    qemu_free(breakpoint);
> +#endif
> +}
> +
> +/* Remove all matching breakpoints. */
> +void cpu_breakpoint_remove_all(CPUState *env, int mask)
> +{
> +#if defined(TARGET_HAS_ICE)
> +    CPUBreakpoint *bp;
> +
> +    for (bp = env->breakpoints; bp != NULL; bp = bp->next)
> +        if (bp->flags & mask)
> +            cpu_breakpoint_remove_by_ref(env, bp);
>  #endif
>  }
>
> @@ -1893,7 +1935,7 @@ int tlb_set_page_exec(CPUState *env, tar
>     target_phys_addr_t addend;
>     int ret;
>     CPUTLBEntry *te;
> -    int i;
> +    CPUWatchpoint *wp;
>     target_phys_addr_t iotlb;
>
>     p = phys_page_find(paddr >> TARGET_PAGE_BITS);
> @@ -1934,8 +1976,8 @@ int tlb_set_page_exec(CPUState *env, tar
>     code_address = address;
>     /* Make accesses to pages with watchpoints go via the
>        watchpoint trap routines.  */
> -    for (i = 0; i < env->nb_watchpoints; i++) {
> -        if (vaddr == (env->watchpoint[i].vaddr & TARGET_PAGE_MASK)) {
> +    for (wp = env->watchpoints; wp != NULL; wp = wp->next) {
> +        if (vaddr == (wp->vaddr & TARGET_PAGE_MASK)) {
>             iotlb = io_mem_watch + paddr;
>             /* TODO: The memory case can be optimized by not trapping
>                reads of pages with a write breakpoint.  */
> @@ -2464,13 +2506,12 @@ static void check_watchpoint(int offset,
>  {
>     CPUState *env = cpu_single_env;
>     target_ulong vaddr;
> -    int i;
> +    CPUWatchpoint *wp;
>
>     vaddr = (env->mem_io_vaddr & TARGET_PAGE_MASK) + offset;
> -    for (i = 0; i < env->nb_watchpoints; i++) {
> -        if (vaddr == env->watchpoint[i].vaddr
> -                && (env->watchpoint[i].type & flags)) {
> -            env->watchpoint_hit = i + 1;
> +    for (wp = env->watchpoints; wp != NULL; wp = wp->next) {
> +        if (vaddr == wp->vaddr && (wp->flags & flags)) {
> +            env->watchpoint_hit = wp;
>             cpu_interrupt(env, CPU_INTERRUPT_DEBUG);
>             break;
>         }
> @@ -2482,40 +2523,40 @@ static void check_watchpoint(int offset,
>    phys routines.  */
>  static uint32_t watch_mem_readb(void *opaque, target_phys_addr_t addr)
>  {
> -    check_watchpoint(addr & ~TARGET_PAGE_MASK, PAGE_READ);
> +    check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_READ);
>     return ldub_phys(addr);
>  }
>
>  static uint32_t watch_mem_readw(void *opaque, target_phys_addr_t addr)
>  {
> -    check_watchpoint(addr & ~TARGET_PAGE_MASK, PAGE_READ);
> +    check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_READ);
>     return lduw_phys(addr);
>  }
>
>  static uint32_t watch_mem_readl(void *opaque, target_phys_addr_t addr)
>  {
> -    check_watchpoint(addr & ~TARGET_PAGE_MASK, PAGE_READ);
> +    check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_READ);
>     return ldl_phys(addr);
>  }
>
>  static void watch_mem_writeb(void *opaque, target_phys_addr_t addr,
>                              uint32_t val)
>  {
> -    check_watchpoint(addr & ~TARGET_PAGE_MASK, PAGE_WRITE);
> +    check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_WRITE);
>     stb_phys(addr, val);
>  }
>
>  static void watch_mem_writew(void *opaque, target_phys_addr_t addr,
>                              uint32_t val)
>  {
> -    check_watchpoint(addr & ~TARGET_PAGE_MASK, PAGE_WRITE);
> +    check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_WRITE);
>     stw_phys(addr, val);
>  }
>
>  static void watch_mem_writel(void *opaque, target_phys_addr_t addr,
>                              uint32_t val)
>  {
> -    check_watchpoint(addr & ~TARGET_PAGE_MASK, PAGE_WRITE);
> +    check_watchpoint(addr & ~TARGET_PAGE_MASK, BP_MEM_WRITE);
>     stl_phys(addr, val);
>  }
>
> Index: b/gdbstub.c
> ===================================================================
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1147,10 +1147,64 @@ void gdb_register_coprocessor(CPUState *
>     }
>  }
>
> +/* GDB breakpoint/watchpoint types */
> +#define GDB_BREAKPOINT_SW        0
> +#define GDB_BREAKPOINT_HW        1
> +#define GDB_WATCHPOINT_WRITE     2
> +#define GDB_WATCHPOINT_READ      3
> +#define GDB_WATCHPOINT_ACCESS    4
> +
> +#ifndef CONFIG_USER_ONLY
> +static const int xlat_gdb_type[] = {
> +    [GDB_WATCHPOINT_WRITE]  = BP_GDB | BP_MEM_WRITE,
> +    [GDB_WATCHPOINT_READ]   = BP_GDB | BP_MEM_READ,
> +    [GDB_WATCHPOINT_ACCESS] = BP_GDB | BP_MEM_ACCESS,
> +};
> +#endif
> +
> +static int gdb_breakpoint_insert(CPUState *env, target_ulong addr,
> +                                 target_ulong len, int type)
> +{
> +    switch (type) {
> +    case GDB_BREAKPOINT_SW ... GDB_BREAKPOINT_HW:
> +        return cpu_breakpoint_insert(env, addr, BP_GDB, NULL);
> +#ifndef CONFIG_USER_ONLY
> +    case GDB_WATCHPOINT_WRITE ... GDB_WATCHPOINT_ACCESS:
> +        return cpu_watchpoint_insert(env, addr, len, xlat_gdb_type[type],
> +                                     NULL);
> +#endif
> +    default:
> +        return -ENOSYS;
> +    }
> +}
> +
> +static int gdb_breakpoint_remove(CPUState *env, target_ulong addr,
> +                                 target_ulong len, int type)
> +{
> +    switch (type) {
> +    case GDB_BREAKPOINT_SW ... GDB_BREAKPOINT_HW:
> +        return cpu_breakpoint_remove(env, addr, BP_GDB);
> +#ifndef CONFIG_USER_ONLY
> +    case GDB_WATCHPOINT_WRITE ... GDB_WATCHPOINT_ACCESS:
> +        return cpu_watchpoint_remove(env, addr, len, xlat_gdb_type[type]);
> +#endif
> +    default:
> +        return -ENOSYS;
> +    }
> +}
> +
> +static void gdb_breakpoint_remove_all(CPUState *env)
> +{
> +    cpu_breakpoint_remove_all(env, BP_GDB);
> +#ifndef CONFIG_USER_ONLY
> +    cpu_watchpoint_remove_all(env, BP_GDB);
> +#endif
> +}
> +
>  static int gdb_handle_packet(GDBState *s, CPUState *env, const char 
> *line_buf)
>  {
>     const char *p;
> -    int ch, reg_size, type;
> +    int ch, reg_size, type, res;
>     char buf[MAX_PACKET_LENGTH];
>     uint8_t mem_buf[MAX_PACKET_LENGTH];
>     uint8_t *registers;
> @@ -1170,8 +1224,7 @@ static int gdb_handle_packet(GDBState *s
>          * because gdb is doing and initial connect and the state
>          * should be cleaned up.
>          */
> -        cpu_breakpoint_remove_all(env);
> -        cpu_watchpoint_remove_all(env);
> +        gdb_breakpoint_remove_all(env);
>         break;
>     case 'c':
>         if (*p != '\0') {
> @@ -1205,8 +1258,7 @@ static int gdb_handle_packet(GDBState *s
>         exit(0);
>     case 'D':
>         /* Detach packet */
> -        cpu_breakpoint_remove_all(env);
> -        cpu_watchpoint_remove_all(env);
> +        gdb_breakpoint_remove_all(env);
>         gdb_continue(s);
>         put_packet(s, "OK");
>         break;
> @@ -1329,44 +1381,6 @@ static int gdb_handle_packet(GDBState *s
>         put_packet(s, "OK");
>         break;
>     case 'Z':
> -        type = strtoul(p, (char **)&p, 16);
> -        if (*p == ',')
> -            p++;
> -        addr = strtoull(p, (char **)&p, 16);
> -        if (*p == ',')
> -            p++;
> -        len = strtoull(p, (char **)&p, 16);
> -        switch (type) {
> -        case 0:
> -        case 1:
> -            if (cpu_breakpoint_insert(env, addr) < 0)
> -                goto breakpoint_error;
> -            put_packet(s, "OK");
> -            break;
> -#ifndef CONFIG_USER_ONLY
> -        case 2:
> -            type = PAGE_WRITE;
> -            goto insert_watchpoint;
> -        case 3:
> -            type = PAGE_READ;
> -            goto insert_watchpoint;
> -        case 4:
> -            type = PAGE_READ | PAGE_WRITE;
> -        insert_watchpoint:
> -            if (cpu_watchpoint_insert(env, addr, type) < 0)
> -                goto breakpoint_error;
> -            put_packet(s, "OK");
> -            break;
> -#endif
> -        default:
> -            put_packet(s, "");
> -            break;
> -        }
> -        break;
> -    breakpoint_error:
> -        put_packet(s, "E22");
> -        break;
> -
>     case 'z':
>         type = strtoul(p, (char **)&p, 16);
>         if (*p == ',')
> @@ -1375,17 +1389,16 @@ static int gdb_handle_packet(GDBState *s
>         if (*p == ',')
>             p++;
>         len = strtoull(p, (char **)&p, 16);
> -        if (type == 0 || type == 1) {
> -            cpu_breakpoint_remove(env, addr);
> -            put_packet(s, "OK");
> -#ifndef CONFIG_USER_ONLY
> -        } else if (type >= 2 || type <= 4) {
> -            cpu_watchpoint_remove(env, addr);
> -            put_packet(s, "OK");
> -#endif
> -        } else {
> +        if (ch == 'Z')
> +            res = gdb_breakpoint_insert(env, addr, len, type);
> +        else
> +            res = gdb_breakpoint_remove(env, addr, len, type);
> +        if (res >= 0)
> +             put_packet(s, "OK");
> +        else if (res == -ENOSYS)
>             put_packet(s, "");
> -        }
> +        else
> +            put_packet(s, "E22");
>         break;
>     case 'q':
>     case 'Q':
> @@ -1506,12 +1519,11 @@ static void gdb_vm_stopped(void *opaque,
>
>     if (reason == EXCP_DEBUG) {
>         if (s->env->watchpoint_hit) {
> -            switch (s->env->watchpoint[s->env->watchpoint_hit - 1].flags &
> -                    (PAGE_READ | PAGE_WRITE)) {
> -            case PAGE_READ:
> +            switch (s->env->watchpoint_hit->flags & BP_MEM_ACCESS) {
> +            case BP_MEM_READ:
>                 type = "r";
>                 break;
> -            case PAGE_READ | PAGE_WRITE:
> +            case BP_MEM_ACCESS:
>                 type = "a";
>                 break;
>             default:
> @@ -1519,10 +1531,9 @@ static void gdb_vm_stopped(void *opaque,
>                 break;
>             }
>             snprintf(buf, sizeof(buf), "T%02x%swatch:" TARGET_FMT_lx ";",
> -                     SIGTRAP, type,
> -                     s->env->watchpoint[s->env->watchpoint_hit - 1].vaddr);
> +                     SIGTRAP, type, s->env->watchpoint_hit->vaddr);
>             put_packet(s, buf);
> -            s->env->watchpoint_hit = 0;
> +            s->env->watchpoint_hit = NULL;
>             return;
>         }
>        tb_flush(s->env);
> Index: b/cpu-defs.h
> ===================================================================
> --- a/cpu-defs.h
> +++ b/cpu-defs.h
> @@ -82,8 +82,6 @@ typedef uint64_t target_phys_addr_t;
>  #define EXCP_HLT        0x10001 /* hlt instruction reached */
>  #define EXCP_DEBUG      0x10002 /* cpu stopped after a breakpoint or 
> singlestep */
>  #define EXCP_HALTED     0x10003 /* cpu is halted (waiting for external 
> event) */
> -#define MAX_BREAKPOINTS 32
> -#define MAX_WATCHPOINTS 32
>
>  #define TB_JMP_CACHE_BITS 12
>  #define TB_JMP_CACHE_SIZE (1 << TB_JMP_CACHE_BITS)
> @@ -142,6 +140,19 @@ typedef struct icount_decr_u16 {
>  } icount_decr_u16;
>  #endif
>
> +typedef struct CPUBreakpoint {
> +    target_ulong pc;
> +    int flags; /* BP_* */
> +    struct CPUBreakpoint *prev, *next;
> +} CPUBreakpoint;
> +
> +typedef struct CPUWatchpoint {
> +    target_ulong vaddr;
> +    target_ulong len;
> +    int flags; /* BP_* */
> +    struct CPUWatchpoint *prev, *next;
> +} CPUWatchpoint;
> +
>  #define CPU_TEMP_BUF_NLONGS 128
>  #define CPU_COMMON                                                      \
>     struct TranslationBlock *current_tb; /* currently executing TB  */  \
> @@ -174,16 +185,11 @@ typedef struct icount_decr_u16 {
>                                                                         \
>     /* from this point: preserved by CPU reset */                       \
>     /* ice debug support */                                             \
> -    target_ulong breakpoints[MAX_BREAKPOINTS];                          \
> -    int nb_breakpoints;                                                 \
> +    CPUBreakpoint *breakpoints;                                         \
>     int singlestep_enabled;                                             \
>                                                                         \
> -    struct {                                                            \
> -        target_ulong vaddr;                                             \
> -        int type; /* PAGE_READ/PAGE_WRITE */                            \
> -    } watchpoint[MAX_WATCHPOINTS];                                      \
> -    int nb_watchpoints;                                                 \
> -    int watchpoint_hit;                                                 \
> +    CPUWatchpoint *watchpoints;                                         \
> +    CPUWatchpoint *watchpoint_hit;                                      \
>                                                                         \
>     struct GDBRegisterState *gdb_regs;                                  \
>                                                                         \
> Index: b/target-i386/translate.c
> ===================================================================
> --- a/target-i386/translate.c
> +++ b/target-i386/translate.c
> @@ -7533,6 +7533,7 @@ static inline void gen_intermediate_code
>     DisasContext dc1, *dc = &dc1;
>     target_ulong pc_ptr;
>     uint16_t *gen_opc_end;
> +    CPUBreakpoint *bp;
>     int j, lj, cflags;
>     uint64_t flags;
>     target_ulong pc_start;
> @@ -7616,9 +7617,9 @@ static inline void gen_intermediate_code
>
>     gen_icount_start();
>     for(;;) {
> -        if (env->nb_breakpoints > 0) {
> -            for(j = 0; j < env->nb_breakpoints; j++) {
> -                if (env->breakpoints[j] == pc_ptr) {
> +        if (unlikely(env->breakpoints)) {
> +            for (bp = env->breakpoints; bp != NULL; bp = bp->next) {
> +                if (bp->pc == pc_ptr) {
>                     gen_debug(dc, pc_ptr - dc->cs_base);
>                     break;
>                 }
> Index: b/target-alpha/translate.c
> ===================================================================
> --- a/target-alpha/translate.c
> +++ b/target-alpha/translate.c
> @@ -2252,6 +2252,7 @@ static always_inline void gen_intermedia
>     target_ulong pc_start;
>     uint32_t insn;
>     uint16_t *gen_opc_end;
> +    CPUBreakpoint *bp;
>     int j, lj = -1;
>     int ret;
>     int num_insns;
> @@ -2274,9 +2275,9 @@ static always_inline void gen_intermedia
>
>     gen_icount_start();
>     for (ret = 0; ret == 0;) {
> -        if (env->nb_breakpoints > 0) {
> -            for(j = 0; j < env->nb_breakpoints; j++) {
> -                if (env->breakpoints[j] == ctx.pc) {
> +        if (unlikely(env->breakpoints)) {
> +            for (bp = env->breakpoints; bp != NULL; bp = bp->next) {
> +                if (bp->pc == ctx.pc) {
>                     gen_excp(&ctx, EXCP_DEBUG, 0);
>                     break;
>                 }
> Index: b/target-arm/translate.c
> ===================================================================
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -8544,6 +8544,7 @@ static inline void gen_intermediate_code
>                                                   int search_pc)
>  {
>     DisasContext dc1, *dc = &dc1;
> +    CPUBreakpoint *bp;
>     uint16_t *gen_opc_end;
>     int j, lj;
>     target_ulong pc_start;
> @@ -8620,9 +8621,9 @@ static inline void gen_intermediate_code
>         }
>  #endif
>
> -        if (env->nb_breakpoints > 0) {
> -            for(j = 0; j < env->nb_breakpoints; j++) {
> -                if (env->breakpoints[j] == dc->pc) {
> +        if (unlikely(env->breakpoints)) {
> +            for (bp = env->breakpoints; bp != NULL; bp = bp->next) {
> +                if (bp->pc == dc->pc) {
>                     gen_set_condexec(dc);
>                     gen_set_pc_im(dc->pc);
>                     gen_exception(EXCP_DEBUG);
> @@ -8675,7 +8676,7 @@ static inline void gen_intermediate_code
>         /* Terminate the TB on memory ops if watchpoints are present.  */
>         /* FIXME: This should be replacd by the deterministic execution
>          * IRQ raising bits.  */
> -        if (dc->is_mem && env->nb_watchpoints)
> +        if (dc->is_mem && env->watchpoints)
>             break;
>
>         /* Translation stops when a conditional branch is enoutered.
> Index: b/target-cris/translate.c
> ===================================================================
> --- a/target-cris/translate.c
> +++ b/target-cris/translate.c
> @@ -3006,10 +3006,11 @@ cris_decoder(DisasContext *dc)
>
>  static void check_breakpoint(CPUState *env, DisasContext *dc)
>  {
> -       int j;
> -       if (env->nb_breakpoints > 0) {
> -               for(j = 0; j < env->nb_breakpoints; j++) {
> -                       if (env->breakpoints[j] == dc->pc) {
> +       CPUBreakpoint *bp;
> +
> +       if (unlikely(env->breakpoints)) {
> +               for (bp = env->breakpoints; bp != NULL; bp = bp->next) {
> +                       if (bp->pc == dc->pc) {
>                                cris_evaluate_flags (dc);
>                                tcg_gen_movi_tl(env_pc, dc->pc);
>                                t_gen_raise_exception(EXCP_DEBUG);
> Index: b/target-m68k/translate.c
> ===================================================================
> --- a/target-m68k/translate.c
> +++ b/target-m68k/translate.c
> @@ -2916,6 +2916,7 @@ gen_intermediate_code_internal(CPUState
>  {
>     DisasContext dc1, *dc = &dc1;
>     uint16_t *gen_opc_end;
> +    CPUBreakpoint *bp;
>     int j, lj;
>     target_ulong pc_start;
>     int pc_offset;
> @@ -2949,9 +2950,9 @@ gen_intermediate_code_internal(CPUState
>     do {
>         pc_offset = dc->pc - pc_start;
>         gen_throws_exception = NULL;
> -        if (env->nb_breakpoints > 0) {
> -            for(j = 0; j < env->nb_breakpoints; j++) {
> -                if (env->breakpoints[j] == dc->pc) {
> +        if (unlikely(env->breakpoints)) {
> +            for (bp = env->breakpoints; bp != NULL; bp = bp->next) {
> +                if (bp->pc == dc->pc) {
>                     gen_exception(dc, dc->pc, EXCP_DEBUG);
>                     dc->is_jmp = DISAS_JUMP;
>                     break;
> @@ -2981,7 +2982,7 @@ gen_intermediate_code_internal(CPUState
>         /* Terminate the TB on memory ops if watchpoints are present.  */
>         /* FIXME: This should be replaced by the deterministic execution
>          * IRQ raising bits.  */
> -        if (dc->is_mem && env->nb_watchpoints)
> +        if (dc->is_mem && env->watchpoints)
>             break;
>     } while (!dc->is_jmp && gen_opc_ptr < gen_opc_end &&
>              !env->singlestep_enabled &&
> Index: b/target-mips/translate.c
> ===================================================================
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -8442,6 +8442,7 @@ gen_intermediate_code_internal (CPUState
>     DisasContext ctx;
>     target_ulong pc_start;
>     uint16_t *gen_opc_end;
> +    CPUBreakpoint *bp;
>     int j, lj = -1;
>     int num_insns;
>     int max_insns;
> @@ -8481,9 +8482,9 @@ gen_intermediate_code_internal (CPUState
>  #endif
>     gen_icount_start();
>     while (ctx.bstate == BS_NONE) {
> -        if (env->nb_breakpoints > 0) {
> -            for(j = 0; j < env->nb_breakpoints; j++) {
> -                if (env->breakpoints[j] == ctx.pc) {
> +        if (unlikely(env->breakpoints)) {
> +            for (bp = env->breakpoints; bp != NULL; bp = bp->next) {
> +                if (bp->pc == ctx.pc) {
>                     save_cpu_state(&ctx, 1);
>                     ctx.bstate = BS_BRANCH;
>                     tcg_gen_helper_0_i(do_raise_exception, EXCP_DEBUG);
> Index: b/target-ppc/translate.c
> ===================================================================
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -6198,6 +6198,7 @@ static always_inline void gen_intermedia
>     target_ulong pc_start;
>     uint16_t *gen_opc_end;
>     int supervisor, little_endian;
> +    CPUBreakpoint *bp;
>     int j, lj = -1;
>     int num_insns;
>     int max_insns;
> @@ -6252,9 +6253,9 @@ static always_inline void gen_intermedia
>     gen_icount_start();
>     /* Set env in case of segfault during code fetch */
>     while (ctx.exception == POWERPC_EXCP_NONE && gen_opc_ptr < gen_opc_end) {
> -        if (unlikely(env->nb_breakpoints > 0)) {
> -            for (j = 0; j < env->nb_breakpoints; j++) {
> -                if (env->breakpoints[j] == ctx.nip) {
> +        if (unlikely(env->breakpoints)) {
> +            for (bp = env->breakpoints; bp != NULL; bp = bp->next) {
> +                if (bp->pc == ctx.nip) {
>                     gen_update_nip(&ctx, ctx.nip);
>                     gen_op_debug();
>                     break;
> Index: b/target-sh4/translate.c
> ===================================================================
> --- a/target-sh4/translate.c
> +++ b/target-sh4/translate.c
> @@ -1799,6 +1799,7 @@ gen_intermediate_code_internal(CPUState
>     DisasContext ctx;
>     target_ulong pc_start;
>     static uint16_t *gen_opc_end;
> +    CPUBreakpoint *bp;
>     int i, ii;
>     int num_insns;
>     int max_insns;
> @@ -1832,9 +1833,9 @@ gen_intermediate_code_internal(CPUState
>         max_insns = CF_COUNT_MASK;
>     gen_icount_start();
>     while (ctx.bstate == BS_NONE && gen_opc_ptr < gen_opc_end) {
> -       if (env->nb_breakpoints > 0) {
> -           for (i = 0; i < env->nb_breakpoints; i++) {
> -               if (ctx.pc == env->breakpoints[i]) {
> +        if (unlikely(env->breakpoints)) {
> +            for (bp = env->breakpoints; bp != NULL; bp = bp->next) {
> +                if (ctx.pc == bp->pc) {
>                    /* We have hit a breakpoint - make sure PC is up-to-date */
>                    tcg_gen_movi_i32(cpu_pc, ctx.pc);
>                    tcg_gen_helper_0_0(helper_debug);
> Index: b/target-sparc/translate.c
> ===================================================================
> --- a/target-sparc/translate.c
> +++ b/target-sparc/translate.c
> @@ -4775,6 +4775,7 @@ static inline void gen_intermediate_code
>     target_ulong pc_start, last_pc;
>     uint16_t *gen_opc_end;
>     DisasContext dc1, *dc = &dc1;
> +    CPUBreakpoint *bp;
>     int j, lj = -1;
>     int num_insns;
>     int max_insns;
> @@ -4812,9 +4813,9 @@ static inline void gen_intermediate_code
>         max_insns = CF_COUNT_MASK;
>     gen_icount_start();
>     do {
> -        if (env->nb_breakpoints > 0) {
> -            for(j = 0; j < env->nb_breakpoints; j++) {
> -                if (env->breakpoints[j] == dc->pc) {
> +        if (unlikely(env->breakpoints)) {
> +            for (bp = env->breakpoints; bp != NULL; bp = bp->next) {
> +                if (bp->pc == dc->pc) {
>                     if (dc->pc != pc_start)
>                         save_state(dc, cpu_cond);
>                     tcg_gen_helper_0_0(helper_debug);
>
>
>
>



-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."