Re: [PATCH v6 1/5] riscv: Introduce support for hardware break/watchpoints

[Rui Qi]

Hi Himanshu,

While testing this series on a platform exposing type-6 triggers, I hit a
failure that seems related to the mcontrol6 timing/hit semantics.

The current execute breakpoint path demuxes the event only by comparing
the programmed breakpoint address with regs->epc:

    if (bp->address == args->regs->epc)
        perf_bp_event(event, args->regs);

On our platform the execute trigger did fire, but the breakpoint
exception was reported with EPC pointing to the next instruction. For
example, the trigger was installed at 0x1062e, while the trap came in
with epc=0x10632. As a result the handler did not call perf_bp_event()
and the breakpoint selftest timed out.

This does not look like an instruction-size matching issue. The trigger
itself matched. The problem is that the kernel identifies the matching
execute trigger only by EPC equality.

For mcontrol6, I think the driver also needs to be careful about the
Sdtrig version exposed by tinfo. With tinfo.version > 0, bit 20 of
mcontrol6 is no longer the old timing bit, and hit1/hit0 encode the
fire timing. The series currently still defines/clears
RISCV_DBTR_MC6_TIMING_BIT and, for watchpoints, only checks
RISCV_DBTR_MC6_HIT_BIT. The execute path does not use hit information
at all.

Would it make sense to make the type-6 path version-aware, or otherwise
avoid relying solely on regs->epc == bp->address?

Thanks,